Bug 78767

Summary: REGRESSION (r90471) - iAd Producer 2.0.1 produces blank pages
Product: WebKit Reporter: Brady Eidson <beidson>
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch v1 - New setting and app-specific quirk sam: review+, beidson: commit-queue-

Description Brady Eidson 2012-02-15 17:57:37 PST
REGRESSION (r90471) - iAd Producer 2.0.1 produces blank pages

The subtle change to "isLoadingInAPISense" did in fact break at least one real world app.

An app-specific quirk patch is coming soon.
Comment 1 Brady Eidson 2012-02-15 18:00:10 PST
Created attachment 127286 [details]
Patch v1 - New setting and app-specific quirk
Comment 2 Sam Weinig 2012-02-15 18:16:58 PST
Comment on attachment 127286 [details]
Patch v1 - New setting and app-specific quirk

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

> Source/WebCore/ChangeLog:12
> +        (WebCore::DocumentLoader::isLoadingInAPISense): Return true if the app needs the quirk
> +          and there are outstanding subresource loads.

Odd spacing here.

> Source/WebCore/loader/DocumentLoader.cpp:452
> +        if (m_frame->settings()->needsIsLoadingInAPISenseQuirk() && !m_subresourceLoaders.isEmpty())
> +            return true;

Why is this not using a check from RuntimeApplicationChecks.h?

> Source/WebCore/page/Settings.h:530
> +        void setNeedsIsLoadingInAPISenseQuirk(bool enabled) { m_needsIsLoadingInAPISenseQuirk = enabled; }
> +        bool needsIsLoadingInAPISenseQuirk() const { return m_needsIsLoadingInAPISenseQuirk; }

Please add a comment indicating what this quirk is.
Comment 3 Brady Eidson 2012-02-15 18:22:11 PST
(In reply to comment #2)
> (From update of attachment 127286 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127286&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        (WebCore::DocumentLoader::isLoadingInAPISense): Return true if the app needs the quirk
> > +          and there are outstanding subresource loads.
> 
> Odd spacing here.
> 
> > Source/WebCore/loader/DocumentLoader.cpp:452
> > +        if (m_frame->settings()->needsIsLoadingInAPISenseQuirk() && !m_subresourceLoaders.isEmpty())
> > +            return true;
> 
> Why is this not using a check from RuntimeApplicationChecks.h?

We've never done version checks from WebCore before.  While it's technically possible, I opted to stick with the pattern we've historically used to do version checks in WebKit/Mac.

> 
> > Source/WebCore/page/Settings.h:530
> > +        void setNeedsIsLoadingInAPISenseQuirk(bool enabled) { m_needsIsLoadingInAPISenseQuirk = enabled; }
> > +        bool needsIsLoadingInAPISenseQuirk() const { return m_needsIsLoadingInAPISenseQuirk; }
> 
> Please add a comment indicating what this quirk is.

Gotcha.

Thanks!
Comment 4 Alexey Proskuryakov 2013-03-16 22:50:29 PDT
Looks like this was committed in <http://trac.webkit.org/changeset/107995>.