1408811 - stylo: XBL important rules are ignored

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Xidorn Quan [:xidorn] UTC+10]

(Note that this is filed for annotating tests in the progress of bug 1407847. The test is not going to fail without patch there, because XUL is still using Gecko backend at the moment.)

The example can be as simple as
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<textbox/>
</window>

It can be seen that the <textbox> has two borders. From inspector, it seems that the inner border is from a <html:input> in its XBL binding.

The inspector shows that there is a rule
> html|*.textbox-input, html|*.textbox-textarea {
> 	margin: 0px !important;
> 	border: none !important;
> 	padding: 0px 1px !important;
> 	background-color: inherit;
> 	color: inherit;
> 	font: inherit;
> }
matches the <html:input>, and clearly it should set the border to nothing. But it is not clear why it doesn't work.

This affects at least
* layout/reftests/forms/textbox/accesskey-2.xul
* layout/reftests/forms/textbox/accesskey-4.xul
because those tests rely on that the <textbox> doesn't show anything.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Investigating. A brief debug shows that we are iterating the declarations in a wrong order, and the UA rules take precedence over the XBL rules somehow.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I eventually understand what's going on there: all important rules in XBL are ignored, because we don't have a cascading level for important XBL rules...

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3beb02e6883dfef017d4d80dbddf18b386ffc92

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Remove XBL as a separate cascading level in Stylo.

In Gecko, we handle XBL rules like author rules everywhere, except that
XBL rules are added and sorted in an independent step, behave as if it
has a separate level.

It is not clear to me why Stylo chose to add a separate level for XBL
rules, but it doesn't seem that there is anything special to do with
XBL rules.

This bug happens because we don't handle XBL important rules which are
handled as part of author rules in Gecko due to lack of the additional
level there. We should just follow what Gecko does here and handle them
all the same.

Review commit: https://reviewboard.mozilla.org/r/191930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/191930/

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1408811 - Update test expectation for this bug.

Review commit: https://reviewboard.mozilla.org/r/191932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/191932/

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8920957 [details]
Remove XBL as a separate cascading level in Stylo.

https://reviewboard.mozilla.org/r/191930/#review197102

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8920958 [details]
Bug 1408811 - Update test expectation for this bug.

https://reviewboard.mozilla.org/r/191932/#review197106

Yay, easy patch + big win :)

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Servo PR

Comment 11

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 2 changesets with 12 changes to 12 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 0f96eff86e7b needs "Bug N" or "No bug" in the commit message.
remote: Xidorn Quan <[e-mail]>
remote: Remove XBL as a separate cascading level in Stylo. r=emilio
remote: 
remote: In Gecko, we handle XBL rules like author rules everywhere, except that
remote: XBL rules are added and sorted in an independent step, behave as if it
remote: has a separate level.
remote: 
remote: It is not clear to me why Stylo chose to add a separate level for XBL
remote: rules, but it doesn't seem that there is anything special to do with
remote: XBL rules.
remote: 
remote: This bug happens because we don't handle XBL important rules which are
remote: handled as part of author rules in Gecko due to lack of the additional
remote: level there. We should just follow what Gecko does here and handle them
remote: all the same.
remote: 
remote: MozReview-Commit-ID: LmqjMdQito2
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/127d2762b087
Update test expectation for this bug. r=emilio

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d22b8c27e4cc
followup - Disable empty-1.xul for stylo windows.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/252a8528c5ab
followup 2 - Swap the annotation of empty-1.xul.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/127d2762b087
https://hg.mozilla.org/mozilla-central/rev/d22b8c27e4cc
https://hg.mozilla.org/mozilla-central/rev/252a8528c5ab