843003 - Children of button does not respond to :hover

Status: RESOLVED DUPLICATE of bug 1089326

(Core :: Layout: Form Controls, defect)

Description

[calle.tornqvist]

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.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Confirmed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130224 Firefox/22.0 ID:20130224031053 CSet: 195e706140d1

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Zero Hawk]

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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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...

Comment 6

[Zero Hawk]

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.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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?

Comment 8

[Mounir Lamouri (: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?

Comment 9

[Boris Zbarsky [: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.

Comment 10

[Mounir Lamouri (:mounir)]

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.

Comment 11

[David Baron :dbaron:]

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).

Comment 12

[David Baron :dbaron:]

Or maybe it's some of the special event handling code in content/html/content/src/HTMLButtonElement.cpp ?

Comment 13

[Boris Zbarsky [:bzbarsky]]

For a start, the code in nsHTMLButtonControlFrame::BuildDisplayList that has the "Do not allow the child subtree to receive events" comment before it.

Comment 14

[Mounir Lamouri (:mounir)]

Attachment: Patch

Boris, let me know if you prefer to not do this review. Feel free to pass it to dbaron too ;)

Comment 15

[Mounir Lamouri (:mounir)]

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?

Comment 16

[David Baron :dbaron:]

(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.

Comment 17

[Mounir Lamouri (:mounir)]

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.

Comment 18

[Mounir Lamouri (:mounir)]

FWIW, this is passing try.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 770791 [details] [diff] [review]
Patch

r=me

Comment 20

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4504582d908c

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Backed out for Marionette bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/86451fc62d82

https://tbpl.mozilla.org/php/getParsedLog.php?id=25172434&tree=Mozilla-Inbound

Comment 22

[Mounir Lamouri (:mounir)]

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?

Comment 24

[Tyler]

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).

Comment 25

[Mats Palmgren (inactive)]

Attachment: unbitrotted

Comment 26

[Mats Palmgren (inactive)]

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

Comment 27

[Malini Das [:mdas] - Away, not checking bugmail]

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.

Comment 28

[Malini Das [:mdas] - Away, not checking bugmail]

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

Comment 29

[Mats Palmgren (inactive)]

Attachment: unbitrotted

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

Comment 30

[erin.janae.anderson]

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!

Comment 31

[erin.janae.anderson]

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!

Comment 32

[Nikita E]

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

Comment 33

[Tyler]

(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.

Comment 34

[Zilmar]

This seems to be a workaround: https://stackoverflow.com/questions/6971350/classhover-not-working-in-firefox

Comment 36

[Mike Taylor [:miketaylr]]

smaug, should this be considered as a dupe of Bug 1089326?

Comment 37

[Olli Pettay [:smaug][[e-mail]]]

yes

Comment 38

[Mike Taylor [:miketaylr]]

thanks.

Comment 39

[u403777]

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).

Comment 40

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 41

[u403777]

(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!