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

Add the ability to queue an element task. #5072

Merged
merged 4 commits into from
Dec 5, 2019

Conversation

dtapuska
Copy link
Contributor

@dtapuska dtapuska commented Nov 8, 2019

The event loop and the document source are attributed to the element's
node document and relevant realm.

Fixes #4980


/images.html ( diff )
/webappapis.html ( diff )

The event loop and the document source are attributed to the element's
node document and relevant realm.

Fixes whatwg#4980
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@dtapuska
Copy link
Contributor Author

dtapuska commented Dec 4, 2019

@domenic, Yoav was asking about the status of the spec changes here. I think this is still pending your review.. Might need to rebase it though.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some tweaks and this looks good to me now. However it does not close #4980 as there are still 200 uses of "queue a task"; this just converts a couple as a sample.

Is the plan to continue iterating in this pull request, or do follow-ups? I'd like to not do 200 pull requests, but somewhere between 1 and 5 seems reasonable...

source Show resolved Hide resolved
Copy link
Contributor Author

@dtapuska dtapuska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning on making it a few pull requests something that is manageable to write and review. Certainly not 200. But I wanted to get the framework in place first. Hmm.. perhaps this should be queue a node task, and then we get it for free for using it for document targets. WDYT?

source Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Dec 4, 2019

Hmm. I guess that seems a little weird as a lot of the tasks don't seem to be "node tasks", i.e. they're not very DOM-related. Perhaps instead we could change the normal "queue a task" to use the document to compute the event loop, if the document is given?

@dtapuska
Copy link
Contributor Author

dtapuska commented Dec 5, 2019

Hmm. I guess that seems a little weird as a lot of the tasks don't seem to be "node tasks", i.e. they're not very DOM-related. Perhaps instead we could change the normal "queue a task" to use the document to compute the event loop, if the document is given?

I think probably using the queue a task will probably be just more natural. So let's leave this as queue an element task.

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

Successfully merging this pull request may close these issues.

Attempt to remove implied-document definition
2 participants