Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[imports]: Parser should not wait on external resources inside imports (bugzilla: 24042) #221

Closed
hayatoito opened this issue Jul 6, 2015 · 1 comment

Comments

@hayatoito
Copy link
Contributor

Title: [imports]: Parser should not wait on external resources inside imports (bugzilla: 24042)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042


comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c0
Elliott Sprehn wrote on 2013-12-09 22:49:17 +0000.

The HTML parser should continue tree building inside an import even if blocked on an external resource.

As a result document.write() should also throw when the currentScript is inside an import since the location of the write is arbitrary.

The result of this is that given:

<script src="slow.js"></script> <script> console.log(1); </script>

While slow.js is being downloaded the entire import tree (1.html, 2.html and 3.html) can be processed and constructed into DOM trees. When it finishes loading the scripts that were queued while tree building these nested imports and the top level import with the console.log should be executed in order. This is conceptually similar to defer@ or async@ on <script>.


comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c1
Boris Zbarsky wrote on 2013-12-10 02:53:10 +0000.

It's not just document.write, is it?

When slow.js runs, can it tell whether 2.html has been loaded? If so, it'll see a somewhat non-deterministic view of the DOM...

I guess @async-(this-is-inserted-to-avoid-notification-in-migration) scripts have some of that behavior too, though.


comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c2
Elliott Sprehn wrote on 2013-12-10 03:00:30 +0000.

(In reply to Boris Zbarsky from comment #1)

It's not just document.write, is it?

When slow.js runs, can it tell whether 2.html has been loaded? If so, it'll
see a somewhat non-deterministic view of the DOM...

I guess @async-(this-is-inserted-to-avoid-notification-in-migration) scripts have some of that behavior too, though.

Yes, @async-(this-is-inserted-to-avoid-notification-in-migration) already has this behavior.

You will always have a non-deterministic view of the main document from an import since it doesn't block the parser as we treat imports like stylesheets not script when parsing. We're extending that behavior to the entire import tree.

The reason to throw in document.write() is that it prevents depending on the tokens being inserted in any particular place. I'm not sure why <script async> doesn't throw when you document.write(), we probably should have made it do so.


comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c3
Boris Zbarsky wrote on 2013-12-10 03:09:11 +0000.

I'm not sure why <script async> doesn't throw when you document.write()

It just silently does nothing instead, presumably because that's less likely to totally break preexisting scripts that someone asyncfiies.

Which is also what document.write() does on a random document that's not being shown, actually.

The only case right now in which document.write() throws is if the document is XML.


comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c4
Elliott Sprehn wrote on 2013-12-10 03:43:25 +0000.

(In reply to Boris Zbarsky from comment #3)

I'm not sure why <script async> doesn't throw when you document.write()

It just silently does nothing instead, presumably because that's less likely
to totally break preexisting scripts that someone asyncfiies.

I'm fine if we make this a no-op in imports to match that.


comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c5
Morrita Hajime wrote on 2014-02-11 01:03:38 +0000.

Renaming the subject to reflect the discussion, and land the fix.

3a2ac11


comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c6
Anne wrote on 2014-02-11 13:14:42 +0000.

Please file bugs https://www.w3.org/Bugs/Public/enter_bug.cgi?product=WHATWG&component=HTML to make sure this gets properly integrated and reviewed by the editor of the HTML specification. Monkey patching without actually involving Ian is not the right way to go about this.


comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c7
Anne wrote on 2014-02-11 13:16:00 +0000.

Also, your fix seems wrong. What if I had changed currentScript to throw on getting?


comment: 8
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c8
Morrita Hajime wrote on 2014-02-11 19:32:24 +0000.

(In reply to Anne from comment #6)

Please file bugs
https://www.w3.org/Bugs/Public/enter_bug.cgi?product=WHATWG&component=HTML
to make sure this gets properly integrated and reviewed by the editor of the
HTML specification. Monkey patching without actually involving Ian is not
the right way to go about this.

(In reply to Anne from comment #7)

Also, your fix seems wrong. What if I had changed currentScript to throw on
getting?

Thanks for the catch. Will file a bug.
The discussion will lead the right way to spec this.
Would you mind to drop some link to the bug which asks this kind of thing?
I'd like to learn how to discuss monkey patch in HTML land.


comment: 9
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c9
Anne wrote on 2014-02-12 11:22:05 +0000.

I see you filed the bug yourself, thanks!

In general we should avoid monkey patching. 1) A specification may not be aware of the patches being applied and invalidate them. 2) Readers of a specification may not be aware of the patches being applied and write code that turns out to be wrong due to patches. 3) If patches start getting applied from multiple sources the mental model quickly becomes too hard and mistakes start creeping in all over.


comment: 10
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c10
Morrita Hajime wrote on 2014-02-12 18:33:41 +0000.

(In reply to Anne from comment #9)

In general we should avoid monkey patching. 1) A specification may not be
aware of the patches being applied and invalidate them. 2) Readers of a
specification may not be aware of the patches being applied and write code
that turns out to be wrong due to patches. 3) If patches start getting
applied from multiple sources the mental model quickly becomes too hard and
mistakes start creeping in all over.

Agreed.

We have some more monkey patches in the import spec.
I filed Bug 24632 to track these.
Although I'm not sure if we can completely get rid of them,
it will be good idea to make these glitches visible.


comment: 11
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c11
Adam Klein wrote on 2014-02-14 23:09:43 +0000.

http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block step 2.3 seems like the thing that needs to be fixed (the ignore-destructive-writes-counter needs to be incremented for imported scripts).


comment: 12
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c12
Morrita Hajime wrote on 2014-02-25 18:46:25 +0000.

Recovering the original summary line.
Will file new one for document.write() thing.


comment: 13
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24042#c13
Morrita Hajime wrote on 2014-04-15 18:16:55 +0000.

Moving this to future considerations.

The source of incompatibility risk (Bug 24808) is killed,
so it's safe to postpone this.

@TakayoshiKochi
Copy link
Member

This has been open without any discussion for some time, and as HTML Imports spec is no longer actively maintained, closing this without any conclusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants