Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ember 3.11: hammertime triggers syle binding warning #90

Open
st-h opened this issue Jul 11, 2019 · 4 comments
Open

ember 3.11: hammertime triggers syle binding warning #90

st-h opened this issue Jul 11, 2019 · 4 comments

Comments

@st-h
Copy link

st-h commented Jul 11, 2019

As already reported to the ember repo here, when ember > 3.11 is used with this addon, code like

<span class="trigger" {{action (toggle '_expanded' this)}}>

will trigger the following warning:

WARNING: Binding style attributes may introduce cross-site scripting vulnerabilities; please ensure that values being bound are properly escaped. For more information, including how to disable this warning, see https://emberjs.com/deprecations/v1.x/#toc_binding-style-attributes. Style affected: "touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;"
@eriktrom
Copy link
Member

@st-h - I believe we're going to deprecate this addon - discussion on that is pending here #86 (more context available here, @runspired or perhaps myself need to ensure that comments in #74 no longer hold true though, for background.)

And I agree with the issue u link to (emberjs/ember.js#18193) - this warning probably should have begun happening before ember 3.11.

given ur using the {{action helper, as a modifier, u can get around this (w/o understanding the root cause in ember) by setting this line to false:

touchActionOnAction: TouchActionOnAction,
. U can also add onclick={{action as well, which is likely easier. The latter will run the ast transformer and prevent style binding at runtime (the former, would remove the touch-action: ... property altogether, but span is not a 'focusable input' element and ios will thus not add 300ms to create a fake hover state (on older iphones, if ur targeting newer iphones only, u could just remove the addon completely per discussion via the first link in this comment, fyi).

(don't quote me verbatim on any of this, we're talking a couple years here in terms of human memory, but I believe those statements are correct, to best of my knowledge, w/o looking again more closely)


leaving this for more context, although i edited the above, so this is mostly to re-iterate why it's like this and why {{on in ember octane is a better choice than {{action -- this addon doesn't support {{on atm though

--

whats happening is that for tags that are not focusable (aka, input tags) and that don't have onclick= (aka, only use a positional {{action modifier, we added this feature a while back for consistency due to confusion that consumers of this lib had due to the different ways {{action could be used.

@st-h
Copy link
Author

st-h commented Jul 12, 2019

@eriktrom thanks a lot for your reply. Honestly, I wasn't adding this addon because I was noticing delays but as a mere measure of preventing such things to show up unexpectedly. I have read through #74 back then and a few stackoverflow questions etc. but my conclusion pretty much was that there still are cases where hammertime would be helpful - but I never took the time to really look into this and its details.

One thing that just comes to my mind is that quite some time back, I stripped all css hover styles on mobile, because they were causing clicks to delay or seemed to introduce the need to click twice (and there is not really a good way to make use of hover at all using touch screens). Personally, I think I will remove this addon for now and see if I run into anything that brings up a need to be fixed. Thanks again for the clarifications.

@eriktrom
Copy link
Member

@st-h - np -- also interesting about the hover css state findings u mention, thanks for sharing.

@st-h
Copy link
Author

st-h commented Jul 12, 2019

@eriktrom pretty simple thing: I just wrap :hover selectors in a media query:

@media (hover: hover) {
  h1:hover {
    color: red;
  }
}

Afaik (even though this was a few years ago) - when iOS safari notices that there is a hover style defined for an element, the first tap activates the hover style (not sure anymore if there just is a delay or a second tap is needed to trigger the actual click). At least removing hover selectors improved the mobile experience for my app. However, I do not really know anything if it still is needed today - I just kept it as it does not actually make much sense to me to define hover styles where hover is not supported. It just seems like a reasonable thing to do.

ref: https://docs.w3cub.com/css/@media/hover/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants