Bug 217315 – Lazy loaded iframe should not lazy load when scripting is disabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Frames

[Rob Buis]

Lazy loaded iframe should not lazy load when scripting is disabled [1].

[1] https://html.spec.whatwg.org/#will-lazy-load-element-steps

[Rob Buis]

Created attachment 410519 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Rob Buis]

Created attachment 410522 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/70207607>

[Rob Buis]

Created attachment 411543 [details]
Patch

[Rob Buis]

Created attachment 414945 [details]
Patch

[Sam Weinig]

Comment on attachment 414945 [details]
Patch

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

> Source/WebCore/html/HTMLIFrameElement.cpp:173
>  
> +    if (document.frame() && !document.frame()->script().canExecuteScripts(NotAboutToExecuteScript))
> +        return false;

For a document without a frame (e.g. one created via new Document() in JS, it seems like this should probably also return false, since script is pretty much implicitly disabled there. 

Would be nice to test that case as well (if its meaningful).

[Rob Buis]

Created attachment 414990 [details]
Patch

[Rob Buis]

Comment on attachment 414945 [details]
Patch

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

>> Source/WebCore/html/HTMLIFrameElement.cpp:173
>> +        return false;
> 
> For a document without a frame (e.g. one created via new Document() in JS, it seems like this should probably also return false, since script is pretty much implicitly disabled there. 
> 
> Would be nice to test that case as well (if its meaningful).

Very good point, thanks! Fixed.

[Darin Adler]

Comment on attachment 414990 [details]
Patch

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

> Source/WebCore/html/HTMLIFrameElement.cpp:172
> +    if (!document.frame() || !document.frame()->script().canExecuteScripts(NotAboutToExecuteScript))

Can we find a way to share this expression with HTMLImageElement::isLazyLoadable? Null check on frame makes this expression long enough that it seems worth while. Maybe.

[Rob Buis]

Created attachment 415018 [details]
Patch

[Rob Buis]

Comment on attachment 414990 [details]
Patch

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

>> Source/WebCore/html/HTMLIFrameElement.cpp:172
>> +    if (!document.frame() || !document.frame()->script().canExecuteScripts(NotAboutToExecuteScript))
> 
> Can we find a way to share this expression with HTMLImageElement::isLazyLoadable? Null check on frame makes this expression long enough that it seems worth while. Maybe.

The best way to share, I think, is to make it even more general, since this construction is used in a few places in HTML. So I added a helper to HTMLElement.

[Rob Buis]

Created attachment 415118 [details]
Patch

[EWS]

Committed r270300: <https://trac.webkit.org/changeset/270300>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415118 [details].