Bug 182695 – Use child text content when determining whether to bail early in running a script

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Domenic Denicola]

Test: http://w3c-test.org/html/semantics/scripting-1/the-script-element/emptyish-script-elements.html

Spec: https://html.spec.whatwg.org/#prepare-a-script step 5

Recent spec change: https://github.com/whatwg/html/pull/3451 (to match Firefox and Edge and be simpler)

The observable difference here is due to whether or not you mark the script as "already-started", preventing future executions when its child text content changes.

[Rob Buis]

Created attachment 400052 [details]
Patch

[youenn fablet]

Comment on attachment 400052 [details]
Patch

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

> Source/WebCore/dom/ScriptElement.cpp:184
> +    const String& sourceText = scriptContent();

s/const String&/auto, scriptContent() is returning a String.
If we need to optimise this a bit, maybe we should add a TextNodeTraversal::isEmpty routine so that we create the string only when executing the script.

[Rob Buis]

Created attachment 400193 [details]
Patch

[Rob Buis]

Comment on attachment 400052 [details]
Patch

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

>> Source/WebCore/dom/ScriptElement.cpp:184
>> +    const String& sourceText = scriptContent();
> 
> s/const String&/auto, scriptContent() is returning a String.
> If we need to optimise this a bit, maybe we should add a TextNodeTraversal::isEmpty routine so that we create the string only when executing the script.

Done. I think it is okay since the majority of the time the script should have non empty content. Also I checked chromium code and they also just get the contents and then test for empty.

[EWS]

Committed r262125: <https://trac.webkit.org/changeset/262125>

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

[Radar WebKit Bug Importer]

<rdar://problem/63597671>