Last Comment Bug 741293 - argument to :dir() and :-moz-locale-dir() is treated case-sensitively; should be case-insensitive
: argument to :dir() and :-moz-locale-dir() is treated case-sensitively; should...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla17
Assigned To: David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
:
Mentors:
Depends on:
Blocks: 478416 562169
  Show dependency treegraph
 
Reported: 2012-04-02 00:50 PDT by John Daggett (:jtd)
Modified: 2012-08-21 06:27 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.14 KB, patch)
2012-04-02 08:29 PDT, David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
no flags Details | Diff | Splinter Review
Treat argument of :-moz-locale-dir() case-insensitively. () (5.12 KB, patch)
2012-08-20 15:26 PDT, David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch)
smontagu: review+
Details | Diff | Splinter Review

Description John Daggett (:jtd) 2012-04-02 00:50:54 PDT
The code in CSSParserImpl::ParsePseudoClassWithIdentArg contains:

  // -moz-locale-dir can only have values of 'ltr' or 'rtl'.
  if (aType == nsCSSPseudoClasses::ePseudoClass_mozLocaleDir) {
    if (!mToken.mIdent.EqualsLiteral("ltr") &&
        !mToken.mIdent.EqualsLiteral("rtl")) {
      return eSelectorParsingStatus_Error; // our caller calls SkipUntil(')')
    }
  }

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSParser.cpp#3419

Those comparisons need to be case-insensitive.
Comment 1 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-04-02 08:26:22 PDT
Yep, should be LowerCaseEqualsLiteral.
Comment 2 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-04-02 08:29:51 PDT
Created attachment 611465 [details] [diff] [review]
patch
Comment 3 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-04-02 08:30:52 PDT
Comment on attachment 611465 [details] [diff] [review]
patch

Er, wait, that's not quite sufficient; need to lowercase what we store too.
Comment 4 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-04-02 08:31:48 PDT
(If someone else wants to finish this that's fine with me.)
Comment 5 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-08-20 14:38:21 PDT
This is higher priority now that bug 562169 landed.  We need to get it in the same release as bug 562169.  Patch coming, once I make sure the tests pass.
Comment 6 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-08-20 15:26:51 PDT
Created attachment 653549 [details] [diff] [review]
Treat argument of :-moz-locale-dir() case-insensitively.  ()
Comment 7 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2012-08-20 22:43:29 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d086edab3616
Comment 8 Ed Morley [:emorley] 2012-08-21 06:27:13 PDT
https://hg.mozilla.org/mozilla-central/rev/d086edab3616

Note You need to log in before you can comment on or make changes to this bug.