Open
Bug 436170
Opened 16 years ago
Updated 2 years ago
CSS:Adjacent sibling selectors don't work on elements added via bindings
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
NEW
People
(Reporter: morac, Unassigned)
References
Details
(Keywords: testcase)
Attachments
(2 files, 4 obsolete files)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
bzbarsky
:
review-
bzbarsky
:
superreview-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0
According to the Dom Inspector, the urlbar contains 3 elements: identity-box, a textbox-input-box and the urlbar-icons. I decided to implement an Internet Explorer type coloring system using the userChrome.css file using the following rules:
#identity-box.verifiedDomain ~ * {
background-color: #F5F6BE !important;
color: #000000 !important;
}
#identity-box.verifiedIdentity ~ * {
background-color: lightgreen !important;
color: #000000 !important;
}
This resulted in the urlbar-icons element changing color, but not the textbox-input-box element. The textbox-input-box element is added via XML binding. So even though it is considered a sibling in the DOM, the CSS style is not applied.
Reproducible: Always
Steps to Reproduce:
Add the following to userChrome.css and restart Firefox.
#identity-box + hbox {
background-color: #F5F6BE !important;
color: #000000 !important;
}
Actual Results:
urlbar-icons in urlbar turns yellow
Expected Results:
textbox-input-box in urlbar should turn yellow
Comment 1•16 years ago
|
||
Comment 2•16 years ago
|
||
Hey...
I was brought to here, from your post on mozillaZine forums:
http://forums.mozillazine.org/viewtopic.php?p=3388784#3388784
I can confirm what you describe, happens when you try out the code.
I'm on Windows XP, Firefox 3 RC2:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906
Firefox/3.0
I'm no coder, but to me it sounds that your argument is reasonable. It should
be possible to have different colored locationbars depending the level of https
security.
I have also uploaded a screenshot showing how it looks with the code enabled.
<= only the 'bookmark star's background has been colored.
But when it all comes down: all I am confirming, is that what you say happens,
does happen.
Comment 3•16 years ago
|
||
Comment 4•16 years ago
|
||
With https://bugzilla.mozilla.org/show_bug.cgi?id=448939 landed, this were the only way to touch these elements at urlbar.
Comment 5•16 years ago
|
||
Maybe it helps... I've noticed the sibling selector just doesn't work if one of them (or both) is an accesible node.
Comment 6•16 years ago
|
||
Comment 7•16 years ago
|
||
reproduced on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre, couldn't find duplicates, confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: testcase
Version: unspecified → Trunk
Comment 8•16 years ago
|
||
It seems that switching to ChildIterator fixes this bug. Looks like this is the right thing to do, too, but please check.
Assignee: nobody → asqueella
Status: NEW → ASSIGNED
Attachment #360016 -
Flags: superreview?(bzbarsky)
Attachment #360016 -
Flags: review?(bzbarsky)
Comment 9•16 years ago
|
||
Comment on attachment 360016 [details] [diff] [review]
make adjacent selector matching code XBL-aware
Given <A/><anonymous node/><B/>, does this handle A + B correctly?
Comment 10•16 years ago
|
||
(In reply to comment #9)
> (From update of attachment 360016 [details] [diff] [review])
> Given <A/><anonymous node/><B/>, does this handle A + B correctly?
Could you define "correctly"?
I think this bug could use a description of what the expected behavior is (and agreement about that description).
Comment 11•16 years ago
|
||
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 360016 [details] [diff] [review] [details])
> > Given <A/><anonymous node/><B/>, does this handle A + B correctly?
>
> Could you define "correctly"?
I would expect it to match B, but I'm not sure if that's what http://www.w3.org/TR/xbl/#selectors says.
Comment 12•16 years ago
|
||
According to <https://developer.mozilla.org/en/XBL/XBL_1.0_Reference/Anonymous_Content#Selectors_and_Scopes>, "style rules using the child, descendant or sibling selectors transparently cross binding scopes and operate on the altered and original content models."
So if the original content model is <A/><B/>, A+B should match B.
Comment 13•16 years ago
|
||
I feel pretty strongly that any content that's actually in the document needs to match or not match based on its siblings in the document, ignoring any XBL anonymous content. CSS styles (e.g., bindings) should not be able to affect the results of CSS selector matching.
I don't have strong opinions about what happens to content that's in the binding... at least not yet.
Comment 14•16 years ago
|
||
Comment on attachment 360016 [details] [diff] [review]
make adjacent selector matching code XBL-aware
What comment 13 said.
Attachment #360016 -
Flags: superreview?(bzbarsky)
Attachment #360016 -
Flags: superreview-
Attachment #360016 -
Flags: review?(bzbarsky)
Attachment #360016 -
Flags: review-
Comment 15•16 years ago
|
||
> Given <A/><anonymous node/><B/>, does this handle A + B correctly?
No, with the patch it doesn't match <B/>. I forgot about <children includes=.../>, so didn't think this situation was possible.
I didn't understand what the XBL2 spec was trying to say, either. I think it tries to say
1) that the selectors must continue to work for non-anonymous content, as if no anonymous content existed (i.e. <B/> should be matched in comment 9's example)
2) that the "+" selector must match an anonymous node if and only if the part before "+" matches the anonymous node right before the context node in the binding. This means that if the final flattened tree looks like this:
<bound-element>
<anonymous node 1/>
<A/> (from <xbl:children/>)
<anonymous node 2/>
<anonymous node 3/>
</bound-element>
.. you can't match any anonymous nodes 1&2 using the '+' selector, but you can match anonymous node #3.
Attachment #324694 -
Attachment is obsolete: true
Attachment #324776 -
Attachment is obsolete: true
Attachment #360004 -
Attachment is obsolete: true
Comment 16•16 years ago
|
||
Comment 15 sounds correct to me in terms of understanding how this should work.
Note that I believe that doesn't address the use case the bug was filed on, right?
Comment 17•16 years ago
|
||
I don't understand for example why this selector actually works: .tabs-newtab-button + stack , which I use to skin the all-tabs stack on FF 3.2
Comment 18•16 years ago
|
||
The testcase from comment 15 seems wrong to me. For "Anonymous child #2 (should be red)" to be red, the selector would have to skip the included button that's between the anonymous nodes. That's neither a match in terms of "altered content" nor "original content".
Comment 19•16 years ago
|
||
> I don't understand for example why this selector actually works:
Because the <xul:toolbarbutton> with class="tabs-newtab-button" is a previous sibling of the <xul:stack> involved? As in, they have the same parent. This parent happens to be an anonymous node in an XBL binding, but they are just plain old DOM kids of their parent, so matching along the DOM tree matches as you see.
Comment 20•16 years ago
|
||
> Note that I believe that doesn't address the use case the bug was filed
> on, right?
Oops, yeah. Sorry about hijacking the bug. I'll have to think about the original use-case more.
> The testcase from comment 15 seems wrong to me.
Right.
Updated•16 years ago
|
Attachment #360004 -
Attachment is obsolete: false
Updated•16 years ago
|
Attachment #360017 -
Attachment is obsolete: true
Comment 21•16 years ago
|
||
Attachment #360016 -
Attachment is obsolete: true
Attachment #361099 -
Flags: superreview?(bzbarsky)
Attachment #361099 -
Flags: review?(bzbarsky)
Comment 22•16 years ago
|
||
v2 implements what I said in comment 2.
I tried to describe how the matching should work with anonymous nodes in source code comments (basically the same as comment 15, plus one deviation from XBL2). The patch also contains the fixed testcase.
I don't see what can be done to address the original use case, so I'm going to clone this bug so that the original issue continues to be open.
Comment 23•16 years ago
|
||
> in comment 2.
"15". Sigh...
Comment 24•16 years ago
|
||
That patch looks like it will assert on native anonymous content matching the rhs of the '+'.
It also looks like it'll do weird things with nested insertion points, since the only check is IsInAnonymousSubtree, without attempts to see whether the two nodes are in the _same_ anonymous subtree.
It also doesn't make :nth-* selectors work, of course. Or :first-* or :last-*. Not sure whether we care, but it seems odd to fix specifically the combinators but not the selectors that do similar things...
(I haven't checked yet whether GetXBLChildNodesFor hands back what we want here, by the way; these are comments off the top of my head.)
It really might be better worth it to wait until sicking cleans up the xbl code and then implement this on top of said cleaner (and more xbl2-like) code.
Comment 25•16 years ago
|
||
Attachment #361099 -
Flags: superreview?(bzbarsky)
Attachment #361099 -
Flags: superreview-
Attachment #361099 -
Flags: review?(bzbarsky)
Attachment #361099 -
Flags: review-
Updated•15 years ago
|
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•