Due to our mishandling the hover quirk, which is annoying. Noticed this when seeing https://github.com/webcompat/web-bugs/issues/12390. Happily enough https://github.com/servo/servo/pull/18876 will fix this, but I think we should land a patch for this to beta.

Confirmed in * Nightly 58 x64 20171014100219 de_DE @ Debian Testing * Beta 57.0b8 x64 20171013042429 de_DE @ Debian Testing Without Stylo: Submenu gets shown when hovering over the link. With Stylo: Nothing happens.

[Tracking Requested - why for this release]: Stylo regression, breaks real webpages.

This was a refactor I did to simplify all this stuff, that fixed this bug pretty much accidentally. Oh well. That's a pretty big patch that can't be uplifted, so I'll write another fix for beta in a second, as soon as my computer finishes pulling the repo.

Still compiling, but I'm 99% sure that this patch will fix this.

It was, quite unsurprisingly, more broken than expected, yay.

[Tracking Requested - why for this release]:

Comment on attachment 8918578 [details] Servo patch. This was r+'d upstream.

Comment on attachment 8918576 [details] Bug 1408672: Test that hover quirk is handled properly during invalidation. https://reviewboard.mozilla.org/r/189410/#review194696

Comment on attachment 8918576 [details] Bug 1408672: Test that hover quirk is handled properly during invalidation. https://reviewboard.mozilla.org/r/189410/#review194698 ::: layout/style/test/test_hover_quirk.html:54 (Diff revision 1) > + width: 100%; > + height: 100%; > + background: red; > + } > + > + #dynamic-test:hover > * { Actually, does this rule invoke the hover quirk? Looking at https://quirks.spec.whatwg.org/#the-active-and-hover-quirk the presence of the ID selector should rule that out. Am I misreading?

Comment on attachment 8918586 [details] [diff] [review] Patch for beta. Review of attachment 8918586 [details] [diff] [review]: ----------------------------------------------------------------- ::: servo/components/selectors/matching.rs @@ +393,5 @@ > let iter = if offset == 0 { > selector.iter() > } else { > + let iter = selector.iter_from(offset); > + local_context.note_position(&iter); Is this needed to fix the bug on beta, or is the other change (disabling the quirk for invalidation) sufficient? If it is needed, can you explain?

(In reply to Cameron McCormack (:heycam) (away 24 Oct – 12 Nov) from comment #10) > ::: layout/style/test/test_hover_quirk.html:54 > (Diff revision 1) > > + width: 100%; > > + height: 100%; > > + background: red; > > + } > > + > > + #dynamic-test:hover > * { > > Actually, does this rule invoke the hover quirk? Looking at > https://quirks.spec.whatwg.org/#the-active-and-hover-quirk the presence of > the ID selector should rule that out. Am I misreading? The whole bug is that we incorrectly think that the hover quirk is enabeld when it shouldn't.

(In reply to Cameron McCormack (:heycam) (away 24 Oct – 12 Nov) from comment #11) > Comment on attachment 8918586 [details] [diff] [review] > Patch for beta. > > Review of attachment 8918586 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: servo/components/selectors/matching.rs > @@ +393,5 @@ > > let iter = if offset == 0 { > > selector.iter() > > } else { > > + let iter = selector.iter_from(offset); > > + local_context.note_position(&iter); > > Is this needed to fix the bug on beta, or is the other change (disabling the > quirk for invalidation) sufficient? If it is needed, can you explain? It is, because matching of partial selector (what this fixes) is also broken in beta, for the same reason described above.

Comment on attachment 8918586 [details] [diff] [review] Patch for beta. Review of attachment 8918586 [details] [diff] [review]: ----------------------------------------------------------------- Please describe in the commit message what this patch is fixing, thanks.

Comment on attachment 8918586 [details] [diff] [review] Patch for beta. Approval Request Comment [Feature/Bug causing the regression]: stylo (bug 1355724 concretely) [User impact if declined]: Broken dynamic changes to some web pages. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: No, though the patch is fairly different. [Needs manual test from QE? If yes, steps to reproduce]: See comment 0's test-case and just open the attached URL and hover over the menu. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: the patch I intend to land to beta is a trivial patch that just disables the quirk during the invalidation process and fixes the invariants in the rest of the code. [String changes made/needed]: none

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/47761f216db8 Test that hover quirk is handled properly during invalidation. r=heycam

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #2) > Confirmed in > * Nightly 58 x64 20171014100219 de_DE @ Debian Testing > * Beta 57.0b8 x64 20171013042429 de_DE @ Debian Testing > Without Stylo: Submenu gets shown when hovering over the link. > With Stylo: Nothing happens. Verified fixed in Nightly 58 x64 20171016100113 de_DE @ Debian Testing. Thanks!

Comment on attachment 8918586 [details] [diff] [review] Patch for beta. Stylo related, Beta57+, let's uplift after it lands on Nightly.

https://hg.mozilla.org/releases/mozilla-beta/rev/64170e5081cd https://hg.mozilla.org/releases/mozilla-beta/rev/51ee30adc28a

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #18) > (In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #2) > > Confirmed in > > * Nightly 58 x64 20171014100219 de_DE @ Debian Testing > > * Beta 57.0b8 x64 20171013042429 de_DE @ Debian Testing > > Without Stylo: Submenu gets shown when hovering over the link. > > With Stylo: Nothing happens. > > Verified fixed in Nightly 58 x64 20171016100113 de_DE @ Debian Testing. Verified fixed in Beta 57.0b9 x64 20171016185129 de_DE @ Debian Testing. Thanks!

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #22) > > Verified fixed in Nightly 58 x64 20171016100113 de_DE @ Debian Testing. > > Verified fixed in Beta 57.0b9 x64 20171016185129 de_DE @ Debian Testing. > Thanks! Thanks a lot for testing it Jan Andre! :)

Managed to reproduce the initial issue on 58.0a1 (2017-10-14). I can confirm that 57.0b13 (20171030163911) and 58.0a1 (2017-10-31) builds are verified fixed, using Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64.