Closed Bug 578714 Opened 15 years ago Closed 15 years ago

Eliminate uses of ToLowerCase/ToUpperCase(nsAString) in layout/style

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b4

People

(Reporter: bzbarsky, Assigned: khuey)

Details

Attachments

(1 file, 4 obsolete files)

Patch
22.72 KB, patch
zwol
: review+
bzbarsky
: superreview+
benjamin
: approval2.0+
Details | Diff | Splinter Review
These are almost certainly wrong and should be using the ASCII versions in nsContentUtils. Would be good to have tests included to make sure we don't regress stuff, too. Example test: <!DOCTYPE html> <head> <meta http-equiv="content-type" content="text/html; charset=utf-8"> <style> body { color: green; } </style> <style> @media prİnt{ body { color: red; } } </style> </head> <body> This should be green when printing <body>
ToUpperCase, if any, should go too.
Summary: Eliminate uses of ToLowerCase(nsAString) in layout/style → Eliminate uses of ToLowerCase/ToUpperCase(nsAString) in layout/style
So there seems to be an unspoken assumption here that any attributes that could be used in selectors are either case sensitive or limited to ASCII. That seems to be correct as far as I can make out from looking through the spec, but I don't see any explicit statement to that effect.
The only time we apply case-folding to attributes is in quirks mode. The quirk is that we do ascii case-folding.
Attached patch Patch (obsolete) — — Splinter Review
There are 7 changes, 6 testcases. I didn't put too much effort into figuring out what causes each nsAttrSelector constructor to be taken. I also haven't touched the one usage of ToUpperCase http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSRuleProcessor.cpp#143 because it's not immediately clear to me what it's doing.
Attachment #458153 - Flags: review?(zweinberg)
There are other ToLowerCase's in layout/style that I didn't touch because they are in debug only code that we probably want to get rid of too.
Interestingly, the language selector case continues to fail (that is, do Unicode aware lowercasing) even after this patch.
> Interestingly, the language selector case continues to fail That's because the compare is done case-insensitively. See the nsCSSPseudoClasses::ePseudoClass_lang case in SelectorMatches. We should change those to use an ascii-insensitive comparator. Can I also recommend producing diffs with 8 lines of context and -p? ;)
(In reply to comment #7) > Can I also recommend producing diffs with 8 lines of context and -p? ;) Heh, forgot to copy my hgrc to that repo. Will fix
Comment on attachment 458153 [details] [diff] [review] Patch ># HG changeset patch ># Parent 3a54d39f2fbb1bde6277c79809001f07e367a512 >Bug 578714: Eliminate Unicode-aware lowercasing in layout/style, use ASCII-only lowercasing. r?=zwol > >diff -r 3a54d39f2fbb content/base/public/nsContentUtils.h >--- a/content/base/public/nsContentUtils.h Sun Jul 18 04:26:38 2010 +0300 >+++ b/content/base/public/nsContentUtils.h Sun Jul 18 01:44:53 2010 -0700 >@@ -1499,12 +1499,14 @@ > /** > * Convert ASCII A-Z to a-z. > */ >+ static void ASCIIToLower(nsAString& aStr); > static void ASCIIToLower(const nsAString& aSource, nsAString& aDest); > > /** > * Convert ASCII a-z to A-Z. > */ > static void ASCIIToUpper(nsAString& aStr); >+ static void ASCIIToUpper(const nsAString& aSource, nsAString& aDest); > > static nsIInterfaceRequestor* GetSameOriginChecker(); > >diff -r 3a54d39f2fbb content/base/src/nsContentUtils.cpp >--- a/content/base/src/nsContentUtils.cpp Sun Jul 18 04:26:38 2010 +0300 >+++ b/content/base/src/nsContentUtils.cpp Sun Jul 18 01:44:53 2010 -0700 >@@ -4955,6 +4955,21 @@ > > /* static */ > void >+nsContentUtils::ASCIIToLower(nsAString& aStr) >+{ >+ PRUnichar* iter = aStr.BeginWriting(); >+ PRUnichar* end = aStr.EndWriting(); >+ while (iter != end) { >+ PRUnichar c = *iter; >+ if (c >= 'A' && c <= 'Z') { >+ *iter = c + ('a' - 'A'); >+ } >+ ++iter; >+ } >+} >+ >+/* static */ >+void > nsContentUtils::ASCIIToLower(const nsAString& aSource, nsAString& aDest) > { > PRUint32 len = aSource.Length(); >@@ -4988,6 +5003,26 @@ > } > } > >+/* static */ >+void >+nsContentUtils::ASCIIToUpper(const nsAString& aSource, nsAString& aDest) >+{ >+ PRUint32 len = aSource.Length(); >+ aDest.SetLength(len); >+ if (aDest.Length() == len) { >+ PRUnichar* dest = aDest.BeginWriting(); >+ const PRUnichar* iter = aSource.BeginReading(); >+ const PRUnichar* end = aSource.EndReading(); >+ while (iter != end) { >+ PRUnichar c = *iter; >+ *dest = (c >= 'a' && c <= 'z') ? >+ c + ('A' - 'a') : c; >+ ++iter; >+ ++dest; >+ } >+ } >+} >+ > PRBool > nsContentUtils::EqualsIgnoreASCIICase(const nsAString& aStr1, > const nsAString& aStr2) >diff -r 3a54d39f2fbb layout/reftests/reftest.list >--- a/layout/reftests/reftest.list Sun Jul 18 04:26:38 2010 +0300 >+++ b/layout/reftests/reftest.list Sun Jul 18 01:44:53 2010 -0700 >@@ -214,6 +214,9 @@ > # -moz-transform/ > include transform/reftest.list > >+# unicode/ (verify that we don't do expend effort doing unicode-aware case checks) >+include unicode/reftest.list >+ > # widget/ > include ../../widget/reftests/reftest.list > >diff -r 3a54d39f2fbb layout/reftests/unicode/reftest.list >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/reftest.list Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,6 @@ >+== unicode-attribute-selector.xhtml unicode-ref.xhtml >+== unicode-element-selector.xhtml unicode-ref.xhtml >+== unicode-lang.xhtml unicode-ref.xhtml >+== unicode-media-query-media-type.xhtml unicode-ref.xhtml >+== unicode-media-query-query.xhtml unicode-ref.xhtml >+== unicode-pseudo-selector.xhtml unicode-ref.xhtml >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-attribute-selector.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-attribute-selector.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,14 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml"> >+<head> >+ <title>Unicode tests - element selectors</title> >+ <style> >+ div[dÄ°r] {color: red} >+ </style> >+</head> >+ >+<body> >+ <div dir='ltr'><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-element-selector.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-element-selector.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,14 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml"> >+<head> >+ <title>Unicode tests - element selectors</title> >+ <style> >+ dÄ°v {color: red} >+ </style> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-lang.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-lang.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,14 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml"> >+<head> >+ <title>Unicode tests - language selector</title> >+ <style> >+ p:lang(hÄ°) {color: red} >+ </style> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-media-query-media-type.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-media-query-media-type.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,16 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml" class="reftest-print"> >+<head> >+ <title>Unicode tests - media query - media type selector</title> >+ <style> >+ @media prÄ°nt { >+ p { color: red } >+ } >+ </style> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-media-query-query.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-media-query-query.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,14 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml" class="reftest-print"> >+<head> >+ <title>Unicode tests - media query - media query text</title> >+ <style> >+ @media print and (mÄ°n-wÄ°dth: 5in) { p {color: red; } } /* y */ >+ </style> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-pseudo-selector.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-pseudo-selector.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,14 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml"> >+<head> >+ <title>Unicode tests - pseudo element selectors</title> >+ <style> >+ p:fÄ°rst-letter {color: red} >+ </style> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/reftests/unicode/unicode-ref.xhtml >--- /dev/null Thu Jan 01 00:00:00 1970 +0000 >+++ b/layout/reftests/unicode/unicode-ref.xhtml Sun Jul 18 01:44:53 2010 -0700 >@@ -0,0 +1,11 @@ >+<?xml version="1.0" encoding="utf-8" ?> >+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> >+<html xmlns="http://www.w3.org/1999/xhtml"> >+<head> >+ <title>Unicode tests - reference rending</title> >+</head> >+ >+<body> >+ <div><p lang="hi">सà¥à¤à¤¾à¤°</p></div> >+</body> >+</html> >diff -r 3a54d39f2fbb layout/style/nsCSSParser.cpp >--- a/layout/style/nsCSSParser.cpp Sun Jul 18 04:26:38 2010 +0300 >+++ b/layout/style/nsCSSParser.cpp Sun Jul 18 01:44:53 2010 -0700 >@@ -1645,7 +1645,7 @@ > return PR_FALSE; > } > // case insensitive from CSS - must be lower cased >- ToLowerCase(mToken.mIdent); >+ nsContentUtils::ASCIIToLower(mToken.mIdent); > mediaType = do_GetAtom(mToken.mIdent); > if (gotNotOrOnly || > (mediaType != nsGkAtoms::_not && mediaType != nsGkAtoms::only)) >@@ -1763,7 +1763,7 @@ > } > > // case insensitive from CSS - must be lower cased >- ToLowerCase(mToken.mIdent); >+ nsContentUtils::ASCIIToLower(mToken.mIdent); > const PRUnichar *featureString; > if (StringBeginsWith(mToken.mIdent, NS_LITERAL_STRING("min-"))) { > expr->mRange = nsMediaExpression::eMin; >@@ -2996,7 +2996,7 @@ > nsAutoString buffer; > buffer.Append(PRUnichar(':')); > buffer.Append(mToken.mIdent); >- ToLowerCase(buffer); >+ nsContentUtils::ASCIIToLower(buffer); > nsCOMPtr<nsIAtom> pseudo = do_GetAtom(buffer); > if (!pseudo) { > mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY); >diff -r 3a54d39f2fbb layout/style/nsCSSStyleRule.cpp >--- a/layout/style/nsCSSStyleRule.cpp Sun Jul 18 04:26:38 2010 +0300 >+++ b/layout/style/nsCSSStyleRule.cpp Sun Jul 18 01:44:53 2010 -0700 >@@ -238,7 +238,7 @@ > MOZ_COUNT_CTOR(nsAttrSelector); > > nsAutoString lowercase; >- ToLowerCase(aAttr, lowercase); >+ nsContentUtils::ASCIIToLower(aAttr, lowercase); > > mCasedAttr = do_GetAtom(aAttr); > mLowercaseAttr = do_GetAtom(lowercase); >@@ -257,7 +257,7 @@ > MOZ_COUNT_CTOR(nsAttrSelector); > > nsAutoString lowercase; >- ToLowerCase(aAttr, lowercase); >+ nsContentUtils::ASCIIToLower(aAttr, lowercase); > > mCasedAttr = do_GetAtom(aAttr); > mLowercaseAttr = do_GetAtom(lowercase); >@@ -393,7 +393,7 @@ > mCasedTag = do_GetAtom(aTag); > > nsAutoString lowercase; >- ToLowerCase(aTag, lowercase); >+ nsContentUtils::ASCIIToLower(aTag, lowercase); > mLowercaseTag = do_GetAtom(lowercase); > } > >diff -r 3a54d39f2fbb layout/style/nsRuleNode.cpp >--- a/layout/style/nsRuleNode.cpp Sun Jul 18 04:26:38 2010 +0300 >+++ b/layout/style/nsRuleNode.cpp Sun Jul 18 01:44:53 2010 -0700 >@@ -4566,7 +4566,7 @@ > nsAutoString lang; > displayData.mLang.GetStringValue(lang); > >- ToLowerCase(lang); >+ nsContentUtils::ASCIIToLower(lang); > visibility->mLanguage = do_GetAtom(lang); > } > }
Attachment #458153 - Attachment is obsolete: true
Attachment #458153 - Flags: review?(zweinberg)
Kyle, would you please re-attach your current version of the patch, rather than pasting it into the bug?
Yeah, once I fix the language selector issue. I tried to obsolete the patch from the "edit as a comment" page and it pasted the entire patch into the bug when I wasn't looking. :-(
(he fell prey to bug 577814)
Attached patch Better patch (obsolete) — — Splinter Review
Better patch. Asking sr from dbaron for the new StringComparator. FWIW, we really should move the ASCIITo[Upper|Lower] out of nsContentUtils and into a more general location.
Attachment #458764 - Flags: superreview?(dbaron)
Attachment #458764 - Flags: review?(zweinberg)
Attached patch hg qrefresh is my friend (obsolete) — — Splinter Review
Attachment #458764 - Attachment is obsolete: true
Attachment #458787 - Flags: superreview?(dbaron)
Attachment #458787 - Flags: review?(zweinberg)
Attachment #458764 - Flags: superreview?(dbaron)
Attachment #458764 - Flags: review?(zweinberg)
Comment on attachment 458787 [details] [diff] [review] hg qrefresh is my friend Why are all of the tests XHTML? Unless there's a specific reason why they need to be, they should be HTML5 instead. In this case, XHTML might actually be doing some harm, as I think that disables case-insensitivity of attribute values (the big nasty special case table near the end of ParseAttributeSelector). You need to test both div[dİr] and div[dİr='ltr'] so you get both code paths through attribute-selector construction. You also need to test div[lang='hİ'] (no match for <div lang="hi">), input[dİsabled] and input[dİsabled='dİsabled'] against <input disabled> (no match in either case), and input[dİsabled='disabled'] against both <input dİsabled="disabled"> and <input dİsabled="DISABLED"> (should only match the first). On the code front, you missed a whole bunch of calls to nsString::EqualsIgnoreCase, which should be ::LowerCaseEqualsASCII or ::LowerCaseEqualsLiteral depending on whether you can put the string constant inside the function call. I'm r-ing because of that. (Unfortunately, a lot of the uses are for :nth-of-type() syntax, which is only looking for the strings "odd", "even", and "n", so you can't use İ to test that. I don't know any characters that cause similar collision problems for d, e, n, o, or v. :-( ) I think you _should_ change the ToUpperCase call in RuleHash_CIHashKey to ASCIIToLower (yes, lower). It's probably not a web-visible bug 'cos this is just a hash function, but the matching CIMatchEntry uses EqualsIgnoreASCIICase, so it'd be more correct that way. I don't know why it's converting to upper case, and since it's for a hash it doesn't actually matter, but class and ID names tend to be lowercase on the web-at-large (I have the impression) so ASCIIToLower would mean we were doing less work. :) I am not really qualified to review new string comparators, but you should absolutely _not_ use the C library tolower(); that's locale-aware.
Attachment #458787 - Flags: review?(zweinberg) → review-
Attached patch Patch (obsolete) — — Splinter Review
Comments addressed, except that I'm not testing attribute value selectors because those call into DOM code at http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSRuleProcessor.cpp#2126 which performs the wrong comparison. We should fix that, but that's outside layout/style so I'll hit it in its bug.
Attachment #458787 - Attachment is obsolete: true
Attachment #460099 - Flags: superreview?(dbaron)
Attachment #460099 - Flags: review?(zweinberg)
Attached patch Patch — — Splinter Review
Attachment #460099 - Attachment is obsolete: true
Attachment #460101 - Flags: superreview?(dbaron)
Attachment #460101 - Flags: review?(zweinberg)
Attachment #460099 - Flags: superreview?(dbaron)
Attachment #460099 - Flags: review?(zweinberg)
Comment on attachment 460101 [details] [diff] [review] Patch > +# unicode/ (verify that we don't do expend effort doing unicode-aware case checks) grammar nit: "don't expend effort" > diff --git a/nsprpub/configure.in.orig b/nsprpub/configure.in.orig > deleted file mode 100644 > diff --git a/toolkit/mozapps/installer/windows/nsis/common.nsh.rej b/toolkit/mozapps/installer/windows/nsis/common.nsh.rej > deleted file mode 100644 > diff --git a/xpcom/components/stub/Makefile.in b/xpcom/components/stub/Makefile.in > deleted file mode 100644 > diff --git a/xpcom/components/stub/nsStaticComponentsStub.cpp b/xpcom/components/stub/nsStaticComponentsStub.cpp > deleted file mode 100644 what's this doing in here? r=zwol
Attachment #460101 - Flags: review?(zweinberg) → review+
I must have qrefreshed crap into a patch higher on my stack.
Comment on attachment 460101 [details] [diff] [review] Patch sr=me, but please make sure to not land that garbage at the end!
Attachment #460101 - Flags: superreview?(dbaron) → superreview+
Attachment #460101 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/9e87c3efc33b
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Target Milestone: --- → mozilla2.0b3
Backed out due to test failures.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Which tests?
Primarily stuff that uses querySelector. E.g. 8546 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Inside Element: :lang() where language comes from the document 8547 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Inside Element: :lang() where language comes from the element 8548 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Inside Element: :lang() where language comes from the element but is a dialect of the language queried 10690 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Disconnected Element: :lang() where language comes from the element 10691 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Disconnected Element: :lang() where language comes from the element but is a dialect of the language queried 12736 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Fragment: :lang() where language comes from the element 12737 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Fragment: :lang() where language comes from the element but is a dialect of the language queried 13216 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Document: :lang() where language comes from the document 13217 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Document: :lang() where language comes from the element 13218 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-1.html | Document: :lang() where language comes from the element but is a dialect of the language queried 16652 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Inside Element: :lang() where language comes from the document 16653 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Inside Element: :lang() where language comes from the element 16654 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Inside Element: :lang() where language comes from the element but is a dialect of the language queried 18805 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Disconnected Element: :lang() where language comes from the element 18806 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Disconnected Element: :lang() where language comes from the element but is a dialect of the language queried 20853 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Fragment: :lang() where language comes from the element 20854 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Fragment: :lang() where language comes from the element but is a dialect of the language queried 21333 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Document: :lang() where language comes from the document 21334 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Document: :lang() where language comes from the element 21335 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug416317-2.html | Document: :lang() where language comes from the element but is a dialect of the language queried
That was mochitest-plain-1/5 Also there was, in 2/5: 5164 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/prototype/test_Prototype.html | testSelectorWithNot - 11 assertions, 1 failures, 0 errors 7565 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/bugs/test_bug42976.html | lang not preserved correctly and in 4/5: 20677 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_parse_rule.html | #a:lang(en) {color: green} - got "rgb(0, 0, 0)", expected "rgb(0, 128, 0)" 20678 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_parse_rule.html | #a:lang( and: REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/talos-slave/mozilla-central-snowleopard-opt-u-reftest/build/reftest/tests/layout/reftests/svg/pseudo-classes-01.svg |
OK. So it's really three failing tests, but run in a bunch of contexts. Specifically, querySelector calls on these strings: ".lang :lang(en)" and ".lang :lang(fr)" should return the right results given this markup: <html lang="en"> <div class="test lang"> <div></div> <div lang="fr"></div> <div lang="en-US"></div> </div> </html> The first query should return the first and third divs, while the second query should return the second one. Does that happen in your build with this patch?
(In reply to comment #26) > OK. So it's really three failing tests, but run in a bunch of contexts. > Specifically, querySelector calls on these strings: ".lang :lang(en)" and > ".lang :lang(fr)" should return the right results given this markup: > > <html lang="en"> > <div class="test lang"> > <div></div> > <div lang="fr"></div> > <div lang="en-US"></div> > </div> > </html> > > The first query should return the first and third divs, while the second query > should return the second one. Does that happen in your build with this patch? Nope, the language selector appears to be completely broken with this patch. :-|
Comment on attachment 460101 [details] [diff] [review] Patch bz points out that >+ PRUnichar l = *lhs++; >+ PRUnichar r = *lhs++; will ruin your day ;-)
http://hg.mozilla.org/mozilla-central/rev/d59ad0384708
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: mozilla2.0b3 → mozilla2.0b4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: