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.

Sign up for GitHub

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

Jump to bottom

Popover with data-trigger="focus" does not work on iOS Safari when used on <a> without class="btn" #15935

Closed
MrOrz opened this issue Feb 26, 2015 · 22 comments · Fixed by #15947
Closed

Popover with data-trigger="focus" does not work on iOS Safari when used on <a> without class="btn" #15935

MrOrz opened this issue Feb 26, 2015 · 22 comments · Fixed by #15947
Labels
confirmed css docs
Milestone
v3.3.4

Comments

@MrOrz
Copy link

MrOrz commented Feb 26, 2015

After #11788 and #14156 the popover with data-trigger="focus" is working very well. However, there is a super weird bug that only happens in iOS safari.

Consider the following 2 <a>, the only difference between them is class="btn".

  <a tabindex="0" role="button" data-toggle="popover" data-trigger="focus"
      class="btn"
      data-content="...">
    Popover anchor w/ class 'btn'
  </a>

  <a tabindex="1" role="button" data-toggle="popover" data-trigger="focus"
      data-content="...">
    Popover anchor w/o class 'btn'
  </a>

The problem is, popover works fine for the first <a>, but not the second.

Hope the recording below will help better illustrating the problem:

1

Demo Plunker: http://embed.plnkr.co/M0NIAPtXOet8ppl2dShP/preview
(Please open using iPhone or iOS simulator. OSX Chrome, Android Chrome, OSX Safari, OSX Firefox works perfectly on both <a>s.)

It seems that the focus event is somehow not triggered in iOS Safari. When I put a breakpoint in the focus event callback, the first <a> invokes the callback as expected while the second <a> does not.

2

No idea why this happens ._.
@twbs-lmvtfy
Copy link

Hi @MrOrz!

You appear to have posted a live example (http://run.plnkr.co/plunks/M0NIAPtXOet8ppl2dShP/), which is always a good first step. However, according to Bootlint, your example has some Bootstrap usage errors, which might potentially be causing your issue:

  • W002: <head> is missing X-UA-Compatible <meta> tag that disables old IE compatibility modes

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!

(Please note that this is a fully automated comment.)

@MrOrz
Copy link
Author

MrOrz commented Feb 26, 2015

X-UA-Compatible added in example and saved :)

The url is the same as before: http://run.plnkr.co/plunks/M0NIAPtXOet8ppl2dShP/

@MrOrz MrOrz changed the title Popover does not work on iOS Safari when used on <a> without class="btn" Popover with data-trigger="focus" does not work on iOS Safari when used on <a> without class="btn" Feb 26, 2015
@cvrebert
Copy link
Collaborator

  • What about iOS 8.1?
  • Does it work if you just add href="#" ?
  • Does it work if you just add style="cursor: pointer" ?

@cvrebert
Copy link
Collaborator

Probable X-Ref: #15277

@cvrebert
Copy link
Collaborator

Confirmed on iOS 8.1.
Without .btn, you need the href="#". .btn doesn't strictly need the href on account of its own cursor: pointer.
As not using .btn is legitimate, I'm just going to add href="#" for the sake of genericity & simplicity, effectively reverting #15277, which was only a very minor nice-to-have anyway.

It seems that the focus event is somehow not triggered in iOS Safari

Yes, iOS Safari is stingy/quirky about elements being clickable/focusable; see https://developer.mozilla.org/en-US/docs/Web/Events/click#Safari_Mobile

@patrickhlauke
Copy link
Member

If at all possible, the best solution is to use an actual <button> element...but yes, failing that, explicitly adding a cursor:pointer would be the cleanest way of doing it, as adding an href="#" will then also require you to preventDefault() the action of the link.

@cvrebert perhaps adding a rule to the CSS along lines of *[role=button] { cursor: pointer; } may be a nice touch?

@cvrebert
Copy link
Collaborator

@patrickhlauke <button> isn't an option because of fucking Safari: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Button#Clicking_and_focus
Way ahead of you on that last thing: necolas/normalize.css#379

@patrickhlauke
Copy link
Member

ah, safari...gotta love it. regarding normalize, as that's been sitting there since october, worth doing our own patch?

@cvrebert
Copy link
Collaborator

Sure, go for it. Let's see what mdo thinks.

@patrickhlauke
Copy link
Member

What would be best? making a change to our local normalize.less, or adding the above to, say, buttons.less ?

@cvrebert
Copy link
Collaborator

Our normalize.less is basically vendored and thus shouldn't be modified.

@cvrebert
Copy link
Collaborator

scaffolding.less might be more appropriate than buttons.less

@patrickhlauke patrickhlauke mentioned this issue Feb 28, 2015
Merged
@XhmikosR
Copy link
Member

XhmikosR commented Mar 2, 2015

@patrickhlauke @cvrebert: Maybe we should add a comment that an upstream patch has been submitted so that we revise this and remove it if added in normalize.css?

@XhmikosR XhmikosR added this to the v3.3.4 milestone Mar 2, 2015
@cvrebert
Copy link
Collaborator

cvrebert commented Mar 2, 2015

@XhmikosR Agreed. Might wanna move your comment to the PR.

patrickhlauke added a commit that referenced this issue Mar 2, 2015
@patrickhlauke
Merge pull request #15947 from patrickhlauke/ios-click-fix-role-button
8e6934c 
iOS fix for role="button" elements; [fixes #15935]
@XhmikosR
Copy link
Member

XhmikosR commented Mar 2, 2015

@patrickhlauke: you should not merge that yet. #15935 (comment)

@patrickhlauke
Copy link
Member

Well I realise that NOW (this is what happens when discussions get fragmented across issues/PRs, sorry)...so, how do I undo this?

@XhmikosR
Copy link
Member

XhmikosR commented Mar 2, 2015

Just add a new commit with the comment.

Well, I didn't have the time to move the comment to the PR. The problem is that there are so many PRs and issues that things like this happen. Just look at the notifications more closely.

@patrickhlauke patrickhlauke mentioned this issue Mar 2, 2015
Merged
@patrickhlauke
Copy link
Member

Done. Will be more careful in future (maybe bad idea to have my notifications set to "firehose"...)

@DavidVII
Copy link

I want to add that this bug is also happening for me on Safari 8.0.4. Not just on iOS. I resolved this by adding the tabindex attribute the .btn element. Safari is having some weird issues trying to trigger the focus event it seems.

@cvrebert
Copy link
Collaborator

@DavidVII That's a very different problem. As already mentioned in the docs, tabindex is always required for dismiss-on-next-click popovers.

@DavidVII
Copy link

@cvrebert My mistake. Figured since it worked on all other browsers except safari that this was related some how.

@cvrebert
Copy link
Collaborator

Though to be fair, Safari is the main reason that the tabindex is necessary; see https://gist.github.com/cvrebert/68659d0333a578d75372

@BradleyCai BradleyCai mentioned this issue Nov 16, 2015
Closed
@jholt1 jholt1 mentioned this issue May 5, 2016
Closed
@asa-odoo asa-odoo mentioned this issue Aug 1, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed css docs
Projects
None yet
Milestone
v3.3.4
Development

Successfully merging a pull request may close this issue.

iOS fix for role="button" elements; [fixes #15935]
6 participants
@MrOrz @XhmikosR @cvrebert @DavidVII @patrickhlauke @twbs-lmvtfy