Bug 3233 – CSS2: Web Kit does not support the :lang pseudo-class

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Dave Hyatt]

* STEPS TO REPRODUCE
1. 10.3.3, Safari 1.2.1
2. Go to http://www.w3.org/International/tests/sec-css-lang-1.html

* RESULTS
Safari fails the test.

* NOTES
There are questionable results on some of the other CSS international tests, but no outright failures:
http://www.w3.org/International/tests/sec-css-lang-2.html
http://www.w3.org/International/tests/sec-css-lang-3.html

[Dave Hyatt]

Apple Bug: <a href="rdar://3611451">rdar://3611451</a>

[Nicholas Shanks]

Created attachment 2340 [details]
:lang() support

Regression tests are CSS3 selector tests #22 and #67:
http://www.w3.org/Style/CSS/Test/CSS3/Selectors/current/html/tests/css3-modsel-22.html

http://www.w3.org/Style/CSS/Test/CSS3/Selectors/current/html/tests/css3-modsel-67.html

[Nicholas Shanks]

Created attachment 2342 [details]
patch rev 1.1

DOMStrings changed to AtomicStrings, QStrings left unchanged due to use of
QString::startsWith() which has no AtomicString/DOMString equivalent. Added
initialization/deletion to constructor and destructor.

[Dave Hyatt]

delete is for objects allocated via new.  You don't need it.  You can also use const AtomicString& to hold 
the result from getAttribute.

[Nicholas Shanks]

Created attachment 2361 [details]
patch rev 1.2

[Nicholas Shanks]

This patch works to a fashion but isn't very good. Basically it emulates *:[lang^="…"] which is not what we 
want - language is not inherited and :lang(e) will match both lang="en" and lang="es" attributes. It doesn't 
support xml:lang="…" either. I am working on a new patch to implement Darin's suggestion of adding 
language to the RenderStyle.

[Ben Gertzfield]

Here's another test case on Wikipedia that uses lang="zh", lang="zh-cn", lang="zh-tw", lang="ja", 
lang="ko" to illustrate glyph variants when browsers correctly switch fonts based on the lang tag:

http://en.wikipedia.org/wiki/Han_unification#Check_your_browser

In Mozilla Firefox 1.0.3 on Mac OS X 10.4.1, each line of the table in that section is rendered differently.  

In Safari 2.0 on OS X 10.4.1, each line is rendered identically, depending on your language setting.  By 
default, the Japanese style of glyph is used for every line of the table; if your language is set to simplified 
Chinese, you get the simplified Chinese glyphs in every line of the table, etc.

[Nicholas Shanks]

Created attachment 2388 [details]
patch rev 2.0 alpha 1

alpha patch, not tested (and doesn't function well). Posted here for
comments/feedback. The main part of this is adding the lang attribute to the
RenderStyle for he element, so that it gets inherited by the element's
children.

This version also fixes the problem with :lang(e) matching both en and es by
stealing the implementation for the [attr|="val"] hyphenated attribute selector
instead of using QString::startsWith(). Might now be possible to eliminate the
QStrings, not sure yet.

[Ben Gertzfield]

Well, the 2.0 alpha 1 patch is definitely alpha. :)  I did my first compile of WebKit, did run-safari, and went 
to this bug's page.  I got a non-styled page, and when I clicked on the link to http://www.w3.org/
International/tests/sec-css-lang-1.html Safari crashed.

I may use rev 1.2 as a base to try to implement the font hints.

[Allan Sandfeld Jensen]

Since the patch is partially based on my patch for KHTML, I should note that 
it has been updated since to handle the problem with "-" and with case.  
There is also a decision to make with case-insensitive matching since KHTML 
now follows the spec but Mozilla does not. 

[Eric Seidel (no email)]

I was doing similar work the other day for the nth-child() and other nth-* selectors (which are also 
recorded in bugzill somewhere).  The parser obviously needs to be augmented to handle these other types 
of selector functions.

[Eric Seidel (no email)]

Comment on attachment 2388 [details]
patch rev 2.0 alpha 1

In general, I think this patch is good.  A couple comments:

1. I think that your additional "string-arg" member variable for CSSSelector
shoudl either be part of a union with simpleSelector, or should be part of a
subclass of CSSSelector.  Either way, we should only pay 4bytes for
simpleSelector and string_arg (bad name, btw) not 8, like we currently are.

Second, I think that NOTFUNCTION can just be replaced by "not(" in the actual
grammar if you like.

No need to add any //kdDebug lines, we're removing those as we go.

When we go about adding nth-* selectors, I don't think we'll keep the string
value in CSSSelector (we might).  We'll probably have bison do the actual an+b
parsing for us, and store a,b on CSSSelector (or more likely a subclass) as
floats perhaps.

[Eric Seidel (no email)]

Nick are you still intersted in finishing this one off?  It's certainly very close to being done!

[Nicholas Shanks]

Eric: no, sorry I have been off working on my own things for six months. If you want to make the changes you suggested yourself and check the patch in, assign it to yourself, I probably won't get on to it in a while.

[Nicholas Shanks]

Replacing the NOTFUNCTION token results in this:

    | ':' 'n' 'o' 't' '(' maybe_space simple_selector ')' {
        CSSParser* p = static_cast<CSSParser*>(parser);
        $$ = p->createFloatingSelector();
        $$->match = CSSSelector::PseudoClass;
        $$->simpleSelector = p->sinkFloatingSelector($7);
        $$->value = atomicString("not(");
    }

and means that it can't be used for anything else one day (CSS4?). I suggest leaving the token in the lexicon but calling it SIMPLEFUNCTION instead.

[Nicholas Shanks]

OK, scratch that, I didn't think it through :-)
The tokenizer cannot distinguish between my concepts of simple and not simple functions.

[Nicholas Shanks]

Created attachment 8719 [details]
patch rev 1.3

This is an update of 1.2 for the current tree and file layout. It doesn't make any changes to the layout engine and adds four bytes to each CSSSelector, but compiles and runs and doesn't screw anything up :-). Will update with more sophisticated patch soon.

[Nicholas Shanks]

Created attachment 8856 [details]
Patch rev 1.4

I have tried to address Eric's first concern, I really have, but due to the fact that AtomicString (and DeprecatedString) both have constructors and destructors, they are not permitted within a union. Everything else is either fixed or open to suggestion and later adjustment (like the variable name).

I am submitting this as the final patch offering for this bug. It really needs to be committed and into the tree as has been here for 12 months now.

Note that this patch is less ambitious than I first wanted: it does not attempt to inherit the language from parent elements nor the HTTP headers, as that was causing instability. I will open a separate bug to track that.

[Eric Seidel (no email)]

Seems fine.  Hyatt should peek at this one before it's landed.

[Dave Hyatt]

Comment on attachment 8856 [details]
Patch rev 1.4

r=me.  Make sure we have bugs covering the remaining issues with :lang.

[Darin Adler]

Landed as r14879.