[Webkit-unassigned] [Bug 50115] Please implement async=false for dynamic script loading (REGRESSION: LABjs is broken)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50115





--- Comment #63 from Tony Gentilcore <tonyg at chromium.org>  2011-02-09 13:55:58 PST ---
(From update of attachment 81749)
Overall the patch looks good, but it looks like the patch should be bounced up against http://html5.org/tools/web-apps-tracker?from=5816&to=5817 to make sure naming and details are as close as possible.


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

> Source/WebCore/dom/ScriptElement.cpp:61
> +    , m_willExecuteInOrder(false)

Since the spec says "Initially, script elements must have this flag set. It is unset...", I think we should default it to true and invert the condition in insertedIntoDocument to unset it instead of setting it.

> Source/WebCore/dom/ScriptElement.h:94
> +    bool m_willExecuteInOrder;

I realize you wrote this patch before the spec update. But now that the spec is updated and calls this "force-async", I think we should use similar names so that future code readers can tie it to the spec easier. I'd suggestion m_forceAsync.

> Source/WebCore/html/HTMLScriptElement.h:72
> +    bool m_forceAsync;

I don't think this should be HTMLScriptElement-specific. The spec talks of the force-async flag for scripts created by the XML parser.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.