Fix the owner document for all element constructors #1168
Conversation
|
We really should check what Edge and WebKit do here, but I think the (B) behavior is the only remotely sane one. |
|
Safari matches Firefox/Chrome, both stable and technology preview. Don't have Edge. |
| data-x="concept-custom-element-definition-local-name">local name</span>.</p></li> | ||
|
|
||
| <li><p>Let <var>document</var> be <var>realm</var>'s <span data-x="concept-realm-global">global | ||
| object</span>'s <span data-x="concept-document-window">newest <code>Document</code> | ||
| object</span>.</p></li> |
There was a problem hiding this comment.
We only use "newest" twice so far. Window's document, document's Window, Window's associated, document's associated, are all more common. (By the way, should we have two separate pointers for those cases? I guess it's clear enough...)
domenic
force-pushed
the
fix-various-owner-documents
branch
from
75da8f1 to
16bbe08
Compare
|
@annevk editorial issues fixed. We can use "current global object" now instead of "current Realm Record's global object" :) |
|
Well, I can test Edge, with a bit of effort. Just have to let me know. On http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4122 Edge throws because Text is not actually constructible in Edge. On http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4143 Edge throws on this line: and the exception is On http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4144 Edge throws on this line: and the exception is On http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4146 Edge actually has the same document all along: the window.open call doesn't create an about:blank document but rather a document with the On http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4147 Edge doesn't reuse the existing Window (and neither does Chrome). Firefox does. |
|
Right, my conclusion when trying to test (although I didn't try as many variations as you did) was that Edge is so far from spec compliance here that we can't really count on their input. |
| @@ -9570,8 +9570,7 @@ interface <dfn>HTMLUnknownElement</dfn> : <span>HTMLElement</span> { };</pre> | |||
| <div w-nodev> | |||
|
|
|||
| <ol> | |||
| <li><p>Let <var>realm</var> be the result of <span>GetFunctionRealm</span>(the currently | |||
| executing <code>HTMLElement</code> function).</p></li> | |||
| <li><p>Let <var>realm</var> be the <span>current Realm Record</span>.</p></li> | |||
There was a problem hiding this comment.
It seems this can become "Let global be [current global object]" given that we only get stuff from the global in two later steps.
|
I think the real question the Edge behavior raises is whether it's web-compatible to simplify things here from what the spec does. For example, Edge's approach to |
|
@DigiTec @travisleithead, thoughts on Edge reusing documents rather than creating a new one? Sounds like you have a rather interesting simplification of the platform that no other browser has. |
domenic
force-pushed
the
fix-various-owner-documents
branch
from
16bbe08 to
f5ac26d
Compare
|
@annevk fixed the HTMLElement constructor up a decent bit more, including making it cross-linkable. Kept that in a separate fixup commit. |
| <li><p>Let <var>element</var> be a new element that implements the <code>HTMLElement</code> | ||
| interface, with no attributes, namespace set to the <span>HTML namespace</span>, local name set | ||
| to <var>localName</var>, and <span>node document</span> set to <var>document</var>.</p></li> | ||
| to <var>definition</var>'s <span data-x="concept-custom-element-definition-local-name">local | ||
| name</span>, and <span>node document</span> set to <var>document</var>.</p></li> |
There was a problem hiding this comment.
Can't we just inline <span>current global object</span>'s <code>Document</code> object here?
There was a problem hiding this comment.
Also, I guess we should use concept-document-window?
|
LGTM with those nits fixed. Unsure whether we should open a separate issue to investigate Edge's simplification more. |
Fixes #1154, which originally covered only the undefined "document" variable in the HTMLElement constructor, but expanded on further investigation to reveal that the spec's current choice of node document for Audio, Image, and Option was strange and not implemented in browsers. While there, cleans up all three of these named constructors to correctly use "create an element," and to be in numbered algorithm form instead of giant-prose-block form; also gives the HTMLElement constructor its own <dfn> and inline some of its steps.
domenic
force-pushed
the
fix-various-owner-documents
branch
from
f5ac26d to
c9d8983
Compare
domenic
added
the
topic: custom elements
Fixes #1154, which originally covered only the undefined "document"
variable in the HTMLElement constructor, but expanded on further
investigation to reveal that the spec's current choice of node document
for Audio, Image, and Option was strange and not implemented in
browsers.
While there, cleans up all three of these named constructors to
correctly use "create an element," and to be in numbered algorithm form
instead of giant-prose-block form.
/cc @bzbarsky @dominiccooney since they were involved in/started that discussion. The end result is probably not worth your time reviewing; the substance of it is the change from #1154 (comment) (A) to (B), which seems to match Chrome and Firefox at least.