| Summary: | Improve http-equiv content-language parsing | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Matt Falkenhagen <falken> | ||||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | REOPENED --- | ||||||||
| Severity: | Normal | CC: | ap, darin, falken, jshin, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Bug Depends on: | 21417, 93571 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Matt Falkenhagen
2012-02-03 01:44:40 PST
To me it seems there are two parts to this: 1) Adding much more extensive test coverage. 2) Fixing any bugs we find. We should do (1) first and land the tests ASAP. Created attachment 126035 [details]
Patch
I uploaded a patch with some tests. I tried to choose reasonable expectations and documented what the spec says and what I've observed other browsers do. The current implementation is failing some of these expectations, which I'm not sure is acceptable or not when adding new tests. On the other hand it didn't quite feel correct to make expectations based only on what the current implementation does. I haven't yet looked into dynamic changes to meta, as Darin suggested in bug 76701. I'll try that next. Also a test for a comma-separated list of languages. Comment on attachment 126035 [details]
Patch
Looks reasonable.
Comment on attachment 126035 [details] Patch Clearing flags on attachment: 126035 Committed r107159: <http://trac.webkit.org/changeset/107159> All reviewed patches have been landed. Closing bug. Thanks for the review. I'll upload the next patch with tests for dynamic changes and comma-separated list. Created attachment 126238 [details]
tests for dynamic changes and comma-separated list
Comment on attachment 126238 [details]
tests for dynamic changes and comma-separated list
Actually, I don't think that this kind of tests belong to fast/text. It's pure DOM/CSS.
cq+ because some tests have already been landed, but please consider moving them.
Comment on attachment 126238 [details] tests for dynamic changes and comma-separated list Clearing flags on attachment: 126238 Committed r107200: <http://trac.webkit.org/changeset/107200> All reviewed patches have been landed. Closing bug. Thanks for the review. Reopening since just tests have landed. Next time please use multiple bug reports. There’s no good reason to use one report for multiple patches when we can file others and have them blocking the original. As far as I recall by re-reading this thread, the remaining task here is part 2) from comment 1: "2) Fixing any bugs we find." I think it may be acceptable to just close this bug and open a new one if bugs are indeed found based on what was landed in comment 5 and comment 10. |