[CSS]bad matching in [attr |= value] selectors if value contains a dash

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
16 years ago
3 years ago

People

(Reporter: glazou, Assigned: glazou)

Tracking

({css2, intl, testcase})

Trunk
css2, intl, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [css] fixed by 83616)

Attachments

(3 attachments)

(Assignee)

Description

16 years ago
Attribute dash-separated value selector has incorrect answer if the tested value
contains a dash. Test case attached.
(Assignee)

Updated

16 years ago
Keywords: correctness, css2, intl, mozilla0.9.1, testcase
(Assignee)

Comment 1

16 years ago
Created attachment 30736 [details]
test case showing bug
(Assignee)

Comment 2

16 years ago
Created attachment 31432 [details] [diff] [review]
patch for 75872
(Assignee)

Comment 3

16 years ago
Proposing patch for this bug. Test case attached to this bug becomes green.
Ian, can you please torture it a little bit ? I think it is ok because I have
other css3 selectors tests that go green but...

Pierre : since vietnamese motorbikes don't have internet access, can you
         r= before your sabbatical ?-) [just kidding, lucky man]
Marc : sr= ?

Thanks
Status: NEW → ASSIGNED
(Assignee)

Comment 4

16 years ago
bug ==> glazman@netscape.com
Assignee: pierre → glazman
Status: ASSIGNED → NEW

Comment 5

16 years ago
I approve! sr=attinasi
- Thanks Daniel

Comment 6

16 years ago
Space characters are not handled with homogeneity:
   <p lang = "en-us">         matches        *[lang |= "en-us"]
   <p lang = " en-us">        matches        *[lang |= "en-us"]
   <p lang = "en-us ">        doesn't match  *[lang |= "en-us"]

Also, with:
    *[lang |= "en-us"]        matching works as described above
    *[lang |= " en-us"]       nothing matches
    *[lang |= "en-us "]       nothing matches

I don't know what the spec says about leading and trailing spaces.  I think both 
should be ignored.

Otherwise, r=pierre.
(Assignee)

Comment 7

16 years ago
Good catch, thanks Pierre. Investigating. If I find no answer, I will push this
issue to the CSS WG.
Btw, setting milestone for 0.9.1.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
# User agents MAY ignore leading and trailing white space in CDATA attribute 
# values (e.g., "   myval   " MAY be interpreted as "myval"). Authors SHOULD 
# NOT declare attribute values with leading or trailing white space.
 -- http://www.w3.org/TR/html4/types.html#h-6.2

We SHOULD NOT ignore trailing spaces in the CSS attributes.

In other words... what Pierre describes is, technically, correct, although I 
would recommend trim()ing the value of the attribute before doing the comparison
(not in the DOM, of course).
DOH! I mean "CSS selectors" not "CSS attributes".

As in, [attr|="x     "] should match <el attr="x     -"/>.

You might want to do the test twice, once without the trim, and if it doesn't
match, then trim it, and try again (if there was anything to trim -- does trim()
tell you if it trimmed anything?).
(Assignee)

Comment 10

16 years ago
Created attachment 34063 [details]
test case ; all paragraph must be green with a correct implementation
(Assignee)

Comment 11

16 years ago
Ian: I think I have found a difficult case...
Given your comments above, the selector

  [attr|=" e"]

should match

  attr="      e "
             ^^
             you find here the selected value

right ?...

I think that the only way to solve this problem is Pierre's proposal : trim
the attribute's value *and* the selected value in selector.

Anyway, this bug needs more discussion between us, and perhaps an escalation to
the CSS WG, and we are friday afternoon, just a few hours before the freeze.
==> marking 0.9.2.
Keywords: mozilla0.9.1 → mozilla0.9.2
Summary: bad matching in [attr |= value] selectors if value contains a dash → [CSS] bad matching in [attr |= value] selectors if value contains a dash
Target Milestone: mozilla0.9.1 → mozilla0.9.2
No, in my opinion

 [attr|=" e"]

should NOT match

  attr="      e "

...because either you completely trim the value, or you don't, you don't do 
bits. I see absolutely no advantage to matching in the above case.

(BTW, the freeze was for major landings, not any bug fix.)
(Assignee)

Comment 13

16 years ago
Ian, I think that this won't be clear from a CSS author's perspective and I
recommend bringing this issue to the CSS WG.

Marc, David, what do you think ?
Daniel: Why isn't it clear? Authors will very quickly learn to not include 
spaces in their dash match attribute selectors... I look forward to replying to
your post in w3c-css-wg. :-)

Updated

16 years ago
Priority: -- → P2
Summary: [CSS] bad matching in [attr |= value] selectors if value contains a dash → [CSS]bad matching in [attr |= value] selectors if value contains a dash
Whiteboard: [css]
(Assignee)

Comment 15

16 years ago
Decision from CSS Working Group : you never trim white space on the CSS side.
White space in attribute values is trimmed during parsing of the XML.
The match is an exact match.

Back to implementation.
...which is basically what I said... :-)

Updated

16 years ago
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Depends on: 83616
This should be fixed with my checkin for bug 83616.

Updated

16 years ago
Whiteboard: [css] → [css] fixed by 83616

Comment 18

16 years ago
Ian, can you test if this is indeed fixed with David's checkin for bug 83616?

Comment 19

16 years ago
attachment 30736 [details] works for me in build 2001070108 win32
attachment 34063 [details] is invalid
marking resolve fixed, Hixie: can you confirm?
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 20

15 years ago
looking at testcase attachment 34063 [details], I still see the bug 

following is the style provided
----------------------------------------
<style type="text/css">
  p { color : red }
  p[lang|="a     "] { color : green }
  p[lang|="b"] { color : green }
  p[lang|="c"] { color : green }
  p[lang|="  d   "] { color : green }
  p[lang|="  en"] { color : green }
  p[lang|="fr"] { color : green }

  p.false { color : green }
  p[lang|="f"] { color : red }

  p.nl {color : green }
  p[lang|=" nl"] { color : red }

  p.se {color : green }
  p[lang|="se "] { color : red }
  </style>
----------------------------------------

the following praragraph text is displayed in 'red'
=========================================
<p lang=" b">This paragraph should be green.</p>
<p lang="c ">This paragraph should be green.</p>
<p lang="  fr-ca  ">Ce paragraphe doit être vert.
     (<span lang="en" style="font-style:italic">This paragraph should be
green</span>)</p>
=======================================

re-opening  bug 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

14 years ago
Target Milestone: mozilla0.9.3 → ---
Madhur, see comment 19.

Resolving fixed again per comment 17.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.