Bug 174691 - Hook up ITP quirks to the needsSiteSpecificQuirks setting
Summary: Hook up ITP quirks to the needsSiteSpecificQuirks setting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 174533 174661
Blocks: 188710
  Show dependency treegraph
 
Reported: 2017-07-20 14:10 PDT by Chris Dumez
Modified: 2018-08-17 15:59 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.13 KB, patch)
2017-07-20 15:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.10 KB, patch)
2017-07-20 16:05 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.10 KB, patch)
2017-07-20 16:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-07-20 14:10:49 PDT
Hook up ITP quirks to the needsSiteSpecificQuirks setting.
Comment 1 Chris Dumez 2017-07-20 15:08:10 PDT
Created attachment 316030 [details]
Patch
Comment 2 Chris Dumez 2017-07-20 16:05:01 PDT
Created attachment 316036 [details]
Patch
Comment 3 Jeremy Jones 2017-07-20 16:41:55 PDT
Comment on attachment 316036 [details]
Patch

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

provisional r+

> Source/WebCore/ChangeLog:8
> +        Hook up ITP quirks to the needsSiteSpecificQuirks setting to make it easier to

nit: easier for
Comment 4 Chris Dumez 2017-07-20 16:57:27 PDT
Created attachment 316043 [details]
Patch
Comment 5 Darin Adler 2017-07-20 18:11:38 PDT
Comment on attachment 316043 [details]
Patch

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

> Source/WebCore/loader/ResourceLoadObserver.cpp:65
> +#if PLATFORM(IOS)
> +    UNUSED_PARAM(page);
> +
> +    // There is currently no way to toggle the needsSiteSpecificQuirks setting on iOS so we always enable
> +    // the site-specific quirks on iOS.
> +    return true;
> +#else

It is really strange that this logic is here rather than in Settings.
Comment 6 Chris Dumez 2017-07-20 18:18:51 PDT
(In reply to Darin Adler from comment #5)
> Comment on attachment 316043 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316043&action=review
> 
> > Source/WebCore/loader/ResourceLoadObserver.cpp:65
> > +#if PLATFORM(IOS)
> > +    UNUSED_PARAM(page);
> > +
> > +    // There is currently no way to toggle the needsSiteSpecificQuirks setting on iOS so we always enable
> > +    // the site-specific quirks on iOS.
> > +    return true;
> > +#else
> 
> It is really strange that this logic is here rather than in Settings.

The setting is disabled on iOS. However, I need the ITP quirk on iOS as well. We could enable the setting on iOS but then it would enable other quirks than ITP on iOS, which would be risky at this point.
Comment 7 WebKit Commit Bot 2017-07-20 18:39:01 PDT
Comment on attachment 316043 [details]
Patch

Clearing flags on attachment: 316043

Committed r219711: <http://trac.webkit.org/changeset/219711>
Comment 8 WebKit Commit Bot 2017-07-20 18:39:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Darin Adler 2017-07-21 09:28:09 PDT
Comment on attachment 316043 [details]
Patch

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

>>> Source/WebCore/loader/ResourceLoadObserver.cpp:65
>>> +#else
>> 
>> It is really strange that this logic is here rather than in Settings.
> 
> The setting is disabled on iOS. However, I need the ITP quirk on iOS as well. We could enable the setting on iOS but then it would enable other quirks than ITP on iOS, which would be risky at this point.

Ah, so site-specific quirks have been non-iOS-only up until now. That seems sloppy and seems worth cleaning up when we have a chance.
Comment 10 Brent Fulgham 2017-10-24 09:04:20 PDT
<rdar://problem/33465715>