[webkit-reviews] review granted: [Bug 170798] Make WebCore::topPrivatelyControlledDomain() return "localhost" for localhost : [Attachment 307309] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted John Wilander
<wilander at apple.com>'s request for review:
Bug 170798: Make WebCore::topPrivatelyControlledDomain() return "localhost" for
localhost
https://bugs.webkit.org/show_bug.cgi?id=170798

Attachment 307309: Patch

https://bugs.webkit.org/attachment.cgi?id=307309&action=review




--- Comment #5 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 307309
  --> https://bugs.webkit.org/attachment.cgi?id=307309
Patch

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

> Source/WebCore/platform/mac/PublicSuffixMac.mm:54
> +    const auto& lowercaseDomain = domain.convertToASCIILowercase();

I guess punycode encoding should not happen here.  This seems correct.

> Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:64
> +    EXPECT_FALSE(isPublicSuffix(""));

Is this the UTF-8 encoding if an uppercase non-ASCII character?  Careful when
committing, because sometimes our tools like webkit-patch UTF-8 encode
non-ASCII characters.  svn  commit doesn't change the characters.
We should probably include a test with an uppercase non-ASCII character and a
test with a lowercase non-ASCII character just for fun.

> Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:90
> +    EXPECT_EQ(String("example.com"),
topPrivatelyControlledDomain("ExamPle.com"));

And let's add a test with a.b.subdomain.Example.com