Bug 227464 - Encoding specified in the 'charset' attribute should have precedence over 'content' attribute
Summary: Encoding specified in the 'charset' attribute should have precedence over 'co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Qiaosong Zhou
URL:
Keywords:
: 227463 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-06-28 15:11 PDT by Qiaosong Zhou
Modified: 2021-06-29 15:26 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.44 KB, patch)
2021-06-28 15:14 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (3.05 KB, patch)
2021-06-28 16:59 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2021-06-29 13:25 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2021-06-29 13:53 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff
Patch (3.82 KB, patch)
2021-06-29 13:57 PDT, Qiaosong Zhou
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Qiaosong Zhou 2021-06-28 15:11:01 PDT
Encoding specified in the 'charset' attribute should have precedence over 'content' attribute
Comment 1 Alex Christensen 2021-06-28 15:12:13 PDT
*** Bug 227463 has been marked as a duplicate of this bug. ***
Comment 2 Qiaosong Zhou 2021-06-28 15:14:11 PDT
Created attachment 432428 [details]
Patch
Comment 3 Alex Christensen 2021-06-28 15:23:32 PDT
Comment on attachment 432428 [details]
Patch

Hooray!  You successfully uploaded a patch.
Comment 4 Qiaosong Zhou 2021-06-28 16:59:41 PDT
Created attachment 432444 [details]
Patch
Comment 5 Geoffrey Garen 2021-06-28 18:03:44 PDT
Comment on attachment 432444 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=432444&action=review

r=me

> Source/WebCore/ChangeLog:9
> +        (WebCore::HTMLMetaCharsetParser::encodingFromMetaAttributes):

It's nice to write a little explanation here about what made the previous logic incorrect, and how the new logic corrects it. Even if the case is pretty simple like this one.
Comment 6 Chris Dumez 2021-06-28 18:30:49 PDT
Comment on attachment 432444 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=432444&action=review

>> Source/WebCore/ChangeLog:9
>> +        (WebCore::HTMLMetaCharsetParser::encodingFromMetaAttributes):
> 
> It's nice to write a little explanation here about what made the previous logic incorrect, and how the new logic corrects it. Even if the case is pretty simple like this one.

Also, for compatibility changes such as this one, please make sure to test the behavior in Chrome and Firefox and provide the results in the ChangeLog. It is important as fixing tests may break compatibility with other browsers in some cases (because other browsers also fail said test).
Comment 7 Chris Dumez 2021-06-28 18:31:18 PDT
(In reply to Chris Dumez from comment #6)
> Comment on attachment 432444 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432444&action=review
> 
> >> Source/WebCore/ChangeLog:9
> >> +        (WebCore::HTMLMetaCharsetParser::encodingFromMetaAttributes):
> > 
> > It's nice to write a little explanation here about what made the previous logic incorrect, and how the new logic corrects it. Even if the case is pretty simple like this one.
> 
> Also, for compatibility changes such as this one, please make sure to test
> the behavior in Chrome and Firefox and provide the results in the ChangeLog.
> It is important as fixing tests may break compatibility with other browsers
> in some cases (because other browsers also fail said test).

For this particular test, I have verified via https://wpt.live/html/syntax/parsing/meta-inhead-insertion-mode.html that it passes on both Firefox and Chrome.
Comment 8 Alex Christensen 2021-06-28 18:37:03 PDT
Comment on attachment 432444 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=432444&action=review

> Source/WebCore/html/parser/HTMLMetaCharsetParser.cpp:115
>                  mode = Charset;

We could also increase performance a little by putting "break;" after this line instead of checking if mode != Charset for the rest of the attributes.
Comment 9 Qiaosong Zhou 2021-06-29 13:25:56 PDT
Created attachment 432516 [details]
Patch
Comment 10 Chris Dumez 2021-06-29 13:28:57 PDT
Comment on attachment 432516 [details]
Patch

Where are the layout test changes?
Comment 11 Qiaosong Zhou 2021-06-29 13:53:04 PDT
Created attachment 432522 [details]
Patch
Comment 12 Chris Dumez 2021-06-29 13:54:08 PDT
Comment on attachment 432522 [details]
Patch

Again this patch does not contain any test or test rebaseline.
Comment 13 Qiaosong Zhou 2021-06-29 13:57:09 PDT
Created attachment 432523 [details]
Patch
Comment 14 EWS 2021-06-29 15:25:37 PDT
Committed r279391 (239256@main): <https://commits.webkit.org/239256@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432523 [details].