Bug 95838 – Remove en_GB from quote handling

• Status: NEW
• Component: CSS

[Nicholas Shanks]

Since en_GB inherits from en, there is no reason to specify identical values for quotations (added in r125476). The second of each of these pairs of lines should be removed, and the expected results regenerated. Sorry, but I can't generate a patch at the moment as I don't have the code on my work computer.


Source/WebCore/rendering/RenderQuote.cpp:

QUOTES_LANG("en",            "\x201c", "\x201d", "\x2018", "\x2019");
QUOTES_LANG("en-GB",         "\x201c", "\x201d", "\x2018", "\x2019");


LayoutTests/fast/css-generated-content/quotes-lang-expected.html:

:root:lang(en),       :not(:lang(en)) > :lang(en)             { quotes: '\201c' '\201d' '\2018' '\2019' }
:root:lang(en-GB),    :not(:lang(en-GB)) > :lang(en-GB)       { quotes: '\201c' '\201d' '\2018' '\2019' }

<q lang="en"><q>en</q></q>
<q lang="en-GB"><q>en-GB</q></q>


LayoutTests/fast/css-generated-content/quotes-lang.html:

<q lang="en"><q>en</q></q>
<q lang="en-GB"><q>en-GB</q></q>

[Alexey Proskuryakov]

Changing RenderQuote.cpp sounds reasonable, but why are you saying that we should remove en_GB from test cases?

[Elliott Sprehn]

Right now the table is a trivial transform of copy/pasting that CSS from the HTML5 spec and running a regex over it. I think it'd be confusing to have lines missing since a diff would be harder to check.

Instead we should auto generate the whole table and the set of parent associations.

[Ian 'Hixie' Hickson]

Why not just use the text from the spec verbatim?

If there's something wrong with the spec, let's fix the spec, please don't intentionally do something different!

[Elliott Sprehn]

(In reply to comment #3)
> Why not just use the text from the spec verbatim?
> 
> If there's something wrong with the spec, let's fix the spec, please don't intentionally do something different!

I'm not sure I understand. Do you mean include that huge block of CSS in the user agent style sheet?

[Ian 'Hixie' Hickson]

Yes.

[Elliott Sprehn]

(In reply to comment #5)
> Yes.

That would require parsing a huge amount of CSS for pages that are likely to never use quotes, and parsing the UA sheet is already considered expensive and memory consuming. It also means that every page has an extra 150 rules to match against.

dhyatt made the original decision to not include a huge CSS sheet in WebKit back on WKBug 3234 and could likely give you more context. He originally mentioned implementing this with CSS, but later said:

"Yes, although I don't want a giant CSS implementation.  This should be done in code."

[Ian 'Hixie' Hickson]

I'm not saying the CSS should be directly used and parsed each time; but why not compile the code from the CSS at compile time?

My concern is just that we not manually be rebuilding this tabel every time it changes. It should just be a trivial copy/paste to update the table, it shouldn't need to involve any careful examination of the rules and optimising things in our head.

(In the case of the en-GB rule, for instance, why don't we just remove the redundant rule from the spec?)

[Elliott Sprehn]

(In reply to comment #7)
> I'm not saying the CSS should be directly used and parsed each time; but why not compile the code from the CSS at compile time?
> 
> My concern is just that we not manually be rebuilding this tabel every time it changes. It should just be a trivial copy/paste to update the table, it shouldn't need to involve any careful examination of the rules and optimising things in our head.
> 
> (In the case of the en-GB rule, for instance, why don't we just remove the redundant rule from the spec?)

Ah, that's already mentioned on https://bugs.webkit.org/show_bug.cgi?id=95835

We should just be downloading and parsing the XML from CLDR for this. I'd rather not parse the HTML5 spec itself. CLDR also has the parent associations so we wouldn't have any of these duplicates.

[Ian 'Hixie' Hickson]

We can provide the CSS rules as an standalone file if that would help.

[Darin Adler]

I don’t think we have inheritance for languages implemented properly. I’d like to see test cases covering these issues. I’m OK with our current implementation, and I don’t really care about one redundant line in an internal table, or with a tiny bit of work needed to update to a new table from a new spec, but I am worried about an incorrect implementation!

If we had the test cases, then we’d be better off.