Closed Bug 843003 Opened 7 years ago Closed Last year

Children of button does not respond to :hover

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1089326

People

(Reporter: calle.tornqvist, Assigned: mounir, NeedInfo)

References

(Blocks 2 open bugs)

Details

(Whiteboard: DUPEME [webcompat])

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.57 Safari/537.17

Steps to reproduce:

Placing a span inside a button with the span having a pseudo-element show on button:hover.

See example: http://jsfiddle.net/Gc4cX/


Actual results:

Pseudo-element does not show.


Expected results:

Pseudo-element should be shown.
Confirmed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130224 Firefox/22.0 ID:20130224031053 CSet: 195e706140d1
Component: Untriaged → Layout
Product: Firefox → Core
Version: 19 Branch → Trunk
Things inside buttons are never in hover state: the button itself steals all mouse events, including mouseover/mouseout.  This also means things inside buttons cannot be clicked, of course.
Whiteboard: DUPEME
Summary: Children of button does not show conditional pseudo element → Children of button does not respond to :hover
Duplicate of this bug: 885698
Hi,

As the author of bug report 885698, which has been marked as a duplicate of this bug 843003, I have a question :-

Is your comment a statement of fact as in 'this is the way it is and it is correct' or is it sarcasm as in 'this is the way it is and its incorrect!.

I would just like clarity as to whether this is a 'bug' and will be fixed (hopefully within my lifetime) or its not considered a bug because it works exactly as designed.

Thanks. 


(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #2)
> Things inside buttons are never in hover state: the button itself steals all
> mouse events, including mouseover/mouseout.  This also means things inside
> buttons cannot be clicked, of course.
It was a statement of fact as in "this is what the implementation does at the moment" (:hover is not matching because the thing inside the button is not in fact in that state), with no value judgment as to whether it's correct or not, because whether it's "correct" depends on the use case...
Hi Boris - thanks for your reply. 

Going forward - being a newbie to this bugzilla format, is there a method to progress this issue or do we wait until someone in a higher power gets assigned to investigate and determine whether or not it is a bug before they can decide on how to progress. If it’s the latter, do you think that I should hold any expectations of a resolution within the foreseeable future.
The best way to progress this issue is to get a specification to cover this behavior...

At a start, though, mounir, what do you think?
Flags: needinfo?(mounir)
(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #7)
> The best way to progress this issue is to get a specification to cover this
> behavior...
> 
> At a start, though, mounir, what do you think?

What would Web Components do in that situation?
Flags: needinfo?(mounir) → needinfo?(bzbarsky)
Whatever the component implementor wants, to some extent: you can have a capturing listener on the component.

That would affect DOM event propagation, but not hit-testing and :hover.
Flags: needinfo?(bzbarsky)
After some testing, the requested behaviour is the behaviour of Webkit, Blink, Presto (FWIW) and IE9. I had issues to reproduce that behaviour on IE10 but I don't know if this is browserstack or a change in behaviour in the engine.

I believe it's probably worth trying to have some kind of consistency and fix this.
Status: UNCONFIRMED → NEW
Component: Layout → Layout: Form Controls
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Fixing this probably requires removing the special code in nsHTMLButtonControlFrame::HandleEvent, and maybe replacing the "if (mRenderer.isDisabled())" bit with something else to do the same thing (or maybe not).
Or maybe it's some of the special event handling code in content/html/content/src/HTMLButtonElement.cpp ?
For a start, the code in nsHTMLButtonControlFrame::BuildDisplayList that has the "Do not allow the child subtree to receive events" comment before it.
Attached patch PatchSplinter Review
Boris, let me know if you prefer to not do this review. Feel free to pass it to dbaron too ;)
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #770791 - Flags: review?(bzbarsky)
This patch gives us a behaviour similar to Webkit and Blink. It is different from Presto (for what it's worth) and Trident in IE9 has the same behaviour regarding hover but not regarding the events propagation.

I remove some code in HTMLButtonElement.cpp that was blocking the event states update of the inner element (comment 12). I also remove the code that was preventing the child display list to be created for the event propogation (comment 13).

The only comment that I did not apply is the following:

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #11)
> Fixing this probably requires removing the special code in
> nsHTMLButtonControlFrame::HandleEvent, and maybe replacing the "if
> (mRenderer.isDisabled())" bit with something else to do the same thing (or
> maybe not).

Removing the entire code does not change anything. I haven't tried with a disabled element but actually, the disabled code should be handled in the content side likely. David, what is the purpose of ::HandleEvent in a frame? Do you have examples where using them is useful?
Flags: needinfo?(dbaron)
Flags: in-testsuite+
(In reply to Mounir Lamouri (:mounir) from comment #15)
> (In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from
> comment #11)
> > Fixing this probably requires removing the special code in
> > nsHTMLButtonControlFrame::HandleEvent, and maybe replacing the "if
> > (mRenderer.isDisabled())" bit with something else to do the same thing (or
> > maybe not).
> 
> Removing the entire code does not change anything. I haven't tried with a
> disabled element but actually, the disabled code should be handled in the
> content side likely. David, what is the purpose of ::HandleEvent in a frame?
> Do you have examples where using them is useful?

Sorry... after making that comment I remembered that much of what the frame HandleEvent methods (which I hate) do is deal with text selection.  So what returning early rather than calling the base class does is (I think) prevent text selection from working.  Though there might be a few other things in frame HandleEvent methods.
Flags: needinfo?(dbaron)
Comment on attachment 770791 [details] [diff] [review]
Patch

I guess I can just ask both of you and one of you guys will do it.
Attachment #770791 - Flags: review?(dbaron)
FWIW, this is passing try.
It seems that the following test is failing:
testing/marionette/client/marionette/www/testAction.html
testing/marionette/client/marionette/tests/unit/test_single_finger_desktop.py
(The two files are related, the actions from the Python script are performed on the HTML.)

As far as I have been able to understand, it seems that the problem is related to .innerHTML update making the click event flaky.

With the patch, the click event is most of the time missed. Without the patch it is all the time received. When .innerHTML is not called or called in a setTimeout(func, 0), the click event is received all the time.

I guess the problem is related to the anonymous content of the button changing in the event handling but I do not know exactly why and how this happen.

David, do you have any chance an understanding of what could produce this bug?
Flags: needinfo?(dbaron)
Target Milestone: mozilla25 → ---
Here is another example http://codepen.io/anon/pen/jfkno that works on other browsers but not in Firefox. Alternative to JSFiddle (since JSFiddle is currently down, unknown when it will be back up).
Blocks: 1076956
Blocks: 1084196
Attached patch unbitrotted (obsolete) — Splinter Review
Malini, do you understand why the patch breaks the Mn tests?
(see also comment 22)
https://tbpl.mozilla.org/?tree=Try&rev=4aab0cedc2fc&jobname=Ubuntu%20VM%2012.04%20try%20debug%20test%20marionette
Flags: needinfo?(mdas)
This test ensures that the right sequence of events are fired. The first test to fail was https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/tests/unit/test_single_finger.py#50, which uses https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/tests/unit/single_finger_functions.py#75 to check if a context menu event was fired on a button. The text on the button affected by the context-menu will change according to the events that were received. I suppose this patch changed that order. Unfortunately, this test doesn't tell you what the actual events fired were, so I filed bug 1085558 about it.

I suggest applying the patch in this bug 1085558 and rerunning the test. You can then see what events were fired and work from there.
Flags: needinfo?(mdas)
The patch for bug 1085558 is on m-i now, if it makes it to m-c, then all you need to do is rebase your patch and run it in try to see the events
Attached patch unbitrottedSplinter Review
Summary of remaining test failures:

Mn on Linux/OSX test_single_finger_desktop.py fails with:
TEST-UNEXPECTED-ERROR | test_single_finger_desktop.py testSingleFingerMouse.test_context_menu | TimeoutException: TimeoutException: wait_for_condition timed out got button1-mousemove-mousedown-contextmenu-mouseup instead of button1-mousemove-mousedown-contextmenu-mouseup-click
etc

M1 on Win32/OSX content/html/content/test/forms/test_input_color_picker_popup.html fails with unexpected assertion:
ASSERTION: we should be in a pseudo-element that is expected to contain elements (:-moz-color-swatch)

M5 on OSX layout/forms/test/test_button_inner_events.html (the test added in this patch) fails with:
button should have received 1 focus events - got 0, expected 1
button should have received 1 keydown events - got 0, expected 1
button should have received 1 keyup events - got 0, expected 1
button should have received 1 keypress events - got 0, expected 1

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=41056f1d32bd
Attachment #8507312 - Attachment is obsolete: true
This issue is still happening. It is causing problems for me where I have text within a button where I want a tooltip to display if a user hover over the text.

I have a codepen that shows how onmouseover works differently in Firefox and Chrome. (http://codepen.io/anon/pen/oYExJr). Here also is a stack overflow problem explaining the issue: http://stackoverflow.com/questions/40943100/onmouseover-not-working-within-an-md-button-in-firefox

Thank You!
Here is another person experiencing this issue too (https://github.com/angular/material/issues/9707) as well as their codepen (http://codepen.io/mhbragg/pen/jrwPXK?editors=1010). Thanks!
Also if you click on some child element of button and positioned outside button, click event will not be triggered on <button>.
Showcase: https://jsfiddle.net/uytqozu4/1/
Similar issue on Stackoverflow: http://stackoverflow.com/questions/8187854/jquery-event-bubbling-on-button-not-working-as-expected-in-firefox
(In reply to corsar89 from comment #32)
> Also if you click on some child element of button and positioned outside
> button, click event will not be triggered on <button>.
> Showcase: https://jsfiddle.net/uytqozu4/1/
> Similar issue on Stackoverflow:
> http://stackoverflow.com/questions/8187854/jquery-event-bubbling-on-button-
> not-working-as-expected-in-firefox

I believe this is a separate issue.
Whiteboard: DUPEME → DUPEME [webcompat]
Duplicate of this bug: 1405718
Flags: webcompat?
smaug, should this be considered as a dupe of Bug 1089326?
Flags: needinfo?(bugs)
yes
Flags: needinfo?(bugs)
thanks.
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → DUPLICATE
Duplicate of bug: 1089326

I am having the same problem:

Targeting span within button tag on hover does not work.
I cannot change border color and pointer on hover.

See second card on the right in this example here: https://codepen.io/anon/pen/JzzEvj

Changing border color and pointer events does work in Chrome and Edge but not in Firefox 65.0.2 (Windows).

Could you try Firefox 66. bug 1089326 is fixed in that release.

(In reply to Olli Pettay [:smaug] from comment #40)

Could you try Firefox 66. bug 1089326 is fixed in that release.

Great, upgrading to Firefox 66 solved this problem. Thanks a lot!

You need to log in before you can comment on or make changes to this bug.