Bug 44865 - REGRESSION (r66156): Sites using AppleConnect for authentication fail to log in.
Summary: REGRESSION (r66156): Sites using AppleConnect for authentication fail to log in.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-08-30 04:09 PDT by Andy Estes
Modified: 2010-08-30 18:02 PDT (History)
0 users

See Also:


Attachments
Patch (1.94 KB, patch)
2010-08-30 04:17 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (3.70 KB, patch)
2010-08-30 17:01 PDT, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2010-08-30 04:09:45 PDT
REGRESSION (r66156): Sites using AppleConnect for authentication fail to log in.
Comment 1 Andy Estes 2010-08-30 04:17:42 PDT
Created attachment 65904 [details]
Patch
Comment 2 Andy Estes 2010-08-30 04:18:59 PDT
<rdar://problem/8366089>
Comment 3 Darin Adler 2010-08-30 10:31:24 PDT
Comment on attachment 65904 [details]
Patch

> +        bool forceFallback = frame->settings()->needsSiteSpecificQuirks() && document()->url().host() == "wdg2.apple.com";

If you put this into an inline function rather than a boolean local variable then you would not be evaluating it if beforeLoadAllowedLoad is true. In the current code you always evaluate this, even if you don't use its result. I think this needs a "why" comment in the code. It can be a brief one.

Is there a guarantee that frame->settings() can't be 0? What if we are in a frame after it is no longer connected to a page? Then frame->settings() will be 0. Are we guaranteed safe from that?

Seems OK, r=me
Comment 4 Andy Estes 2010-08-30 17:01:09 PDT
Created attachment 65980 [details]
Patch
Comment 5 Andy Estes 2010-08-30 17:02:25 PDT
Darin, based on our conversation this morning, here is an updated patch that keys off the plug-in MIME type rather than the URL and works by lower-casing the plug-in parameters rather than falling back to the embed.
Comment 6 Darin Adler 2010-08-30 17:40:02 PDT
Comment on attachment 65980 [details]
Patch

> +    if (frame && frame->settings()->needsSiteSpecificQuirks() && mimeType == "application/x-snkp") {

Since MIME types are not case sensitive it would be better to use equalIgnoringCase.

You’ll also need to update StringsNotToBeLocalized.txt to add this new non-localized string.
Comment 7 Andy Estes 2010-08-30 18:02:30 PDT
Committed http://trac.webkit.org/changeset/66437.