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.

Sign up for GitHub

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

Jump to bottom

Selection and document.open() #15812

Merged
merged 1 commit into from Mar 14, 2019
Merged

Selection and document.open() #15812

merged 1 commit into from Mar 14, 2019

Conversation

annevk
Copy link
Member

@annevk annevk commented Mar 13, 2019

Chrome fails this, interestingly enough, but Firefox and Safari both pass.

cc @bzbarsky @TimothyGu @domenic

@wpt-pr-bot wpt-pr-bot requested a review from rniwa March 13, 2019 13:14
@bzbarsky
Copy link
Contributor

Which test assertion does Chrome fail?

@annevk
Copy link
Member Author

annevk commented Mar 13, 2019

That the object must be the same. Presumably also the final assertion as it passes the current test.

domenic
domenic approved these changes Mar 13, 2019
View reviewed changes
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.

LGTM. Curious result. A Chrome bug would be great :).

@annevk
Copy link
Member Author

annevk commented Mar 14, 2019

https://bugs.chromium.org/p/chromium/issues/detail?id=941913

@annevk annevk merged commit 0ea8f9e into master Mar 14, 2019
@annevk annevk deleted the annevk/selection-doc-open branch March 14, 2019 08:04
@yosinch
Copy link
Contributor

yosinch commented Mar 15, 2019

As of today, Blink's Selection comes from document.getSelection(). window.getSelection() calls document.getSelection(). This is reason why Blink have new selection object after document.open().

To fix this issue, we need to get rid getSelection() from DocumentOrShadowRoot interface.

@annevk
Copy link
Member Author

annevk commented Mar 15, 2019

I don't really see why that matters here. Neither the Window nor the Document object change here. I also strongly suspect that setup is the same in all implementations as it's what https://w3c.github.io/selection-api/#dom-window-getselection requires.

@TimothyGu
Copy link
Member

TimothyGu commented May 4, 2019 •
edited

Currently, Blink has an explicit call to reset the Selection object in document.open(). That call was added in chromium/chromium@bff9c92 for https://crbug.com/683066 in order to pass the older version of selection/Document-open.html, which was changed in this PR. It seems like the fix should just be to revert that commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domenic domenic domenic approved these changes

@rniwa rniwa Awaiting requested review from rniwa

Assignees

@rniwa rniwa

Labels
selection
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@annevk @bzbarsky @yosinch @TimothyGu @domenic @rniwa @wpt-pr-bot