Bug 159049 – Enable window.open() for existing versions of Secret Society (compatibility)

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[John Wilander]

The Secret Society Hidden Mystery app has a broken version check treating iOS 10 as iOS 1 on iPads. Therefore it believes it can use window.open() in a tap handler. We should allow the existing versions of the app to do this to not break them.

[John Wilander]

rdar://problem/26528349

[John Wilander]

Created attachment 281893 [details]
Patch

[Andy Estes]

Comment on attachment 281893 [details]
Patch

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

> Source/WTF/wtf/spi/darwin/dyldSPI.h:40
> +#define DYLD_IOS_VERSION_3_0  0x00030000
> +#define DYLD_IOS_VERSION_4_2  0x00040200
> +#define DYLD_IOS_VERSION_5_0  0x00050000
> +#define DYLD_IOS_VERSION_6_0  0x00060000
> +#define DYLD_IOS_VERSION_7_0  0x00070000
> +#define DYLD_IOS_VERSION_9_0  0x00090000
> +#define DYLD_IOS_VERSION_10_0 0x000A0000

We try to avoid lining things up on column boundaries like this. The problem with doing this is that we'd no longer get useful blame information for lines 34-39.

> Source/WebCore/page/DOMWindow.cpp:357
> -
> +    

Please remove this.

> Source/WebCore/page/DOMWindow.cpp:366
> +    

And this.

[John Wilander]

Created attachment 281909 [details]
Patch

[John Wilander]

Created attachment 281910 [details]
Patch

[John Wilander]

(In reply to comment #3)
> Comment on attachment 281893 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281893&action=review
> 
> > Source/WTF/wtf/spi/darwin/dyldSPI.h:40
> > +#define DYLD_IOS_VERSION_3_0  0x00030000
> > +#define DYLD_IOS_VERSION_4_2  0x00040200
> > +#define DYLD_IOS_VERSION_5_0  0x00050000
> > +#define DYLD_IOS_VERSION_6_0  0x00060000
> > +#define DYLD_IOS_VERSION_7_0  0x00070000
> > +#define DYLD_IOS_VERSION_9_0  0x00090000
> > +#define DYLD_IOS_VERSION_10_0 0x000A0000
> 
> We try to avoid lining things up on column boundaries like this. The problem
> with doing this is that we'd no longer get useful blame information for
> lines 34-39.

I recall you telling me this previously. :) Fixed.

> 
> > Source/WebCore/page/DOMWindow.cpp:357
> > -
> > +    
> 
> Please remove this.
> 
> > Source/WebCore/page/DOMWindow.cpp:366
> > +    
> 
> And this.

Fixed them both and another stray tab.

Thanks!

[WebKit Commit Bot]

Comment on attachment 281910 [details]
Patch

Clearing flags on attachment: 281910

Committed r202380: <http://trac.webkit.org/changeset/202380>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> Comment on attachment 281910 [details]
> Patch
> 
> Clearing flags on attachment: 281910
> 
> Committed r202380: <http://trac.webkit.org/changeset/202380>

Follow-up build fix for internal iOS 9.x SDK builds:

Committed r202400: <http://trac.webkit.org/changeset/r202400>

[John Wilander]

Thanks, Dave!

[Alexey Proskuryakov]

> Follow-up build fix for internal iOS 9.x SDK builds:
> 
> Committed r202400: <http://trac.webkit.org/changeset/r202400>

ChangeLog for the build fix says "Define DYLD_IOS_VERSION_10_0 when building on internal SDKs older than iOS 10" - but the patch implemented something different. It defined the macro when targeting a pre-iOS 10 SDK. 

To check for SDK version, use MAX_ALLOWED, not MIN_REQUIRED.

[Darin Adler]

I believe we should remove the setting allowWindowOpenWithoutUserGesture because it is a duplicate of the already-existing setting javaScriptCanOpenWindowsAutomatically.

[Darin Adler]

Maybe Joe wants to tackle this as part of his cleanup?