Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upSelection DOM interface (but not actual UI selections) #25674
Conversation
highfive
commented
Feb 2, 2020
|
Heads up! This PR modifies the following files:
|
|
@bors-servo try=wpt |
Selection DOM interface (but not actual UI selections) This is work towards #7492. Things that aren't done here: - The spec doesn't describe what tests/wpt/metadata/selection/script-and-style-elements.html.ini wants, and what it wants seems to require some sort of layout query. - #25673 means I'm not sure about some tests being OK. - I haven't checked that the selectionchange event is actually working, and it has no WPT tests other than that the handler exists. - Actual UI interactions are not present at all; selection.rs has a few TODOs saying methods what I understand the UI should call for what actions, but there are a lot of fine points here that I don't know about.
|
|
|
Nothing too unexpected here...
|
|
@bors-servo try=wpt |
Selection DOM interface (but not actual UI selections) This is work towards #7492. I put new tests in the mozilla-specific directory rather than the main tree because I'm not sure how upstreamable they are; I'd like opinions on that point. This adds a new exposed interface, which I understand from tests/mozilla/interfaces.html is something requiring specific approval from someone allowed to give that approval. Things that aren't done here: - The spec doesn't describe what selection/script-and-style-elements.html wants, and what it wants seems to require some sort of layout query. - Actual UI interactions are not present at all; selection.rs has a few TODOs saying which methods I believe the eventual UI code should call for what actions, but there are a lot of fine points here that I don't know about. - I haven't touched Node; you can ask if a node's in the Selection's Range the same way you'd ask about any other Range, but there's not a faster path just for selection layout.
|
|
I've never seen this one before. It looks like the test is iterating over many of image pixels one at a time in Javascript, so maybe it's susceptible to timeouts if it's not getting as many CPU cycles. It isn't touching selections or ranges. |
|
Great work! |
| return Err(Error::IndexSize); | ||
| } | ||
|
|
||
| // let is_old_anchor_before_or_equal; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| focus_offset: u32, | ||
| ) -> ErrorResult { | ||
| // Step 1 | ||
| if anchor_node.is_doctype() || focus_node.is_doctype() { |
This comment has been minimized.
This comment has been minimized.
jdm
Feb 11, 2020
Member
Presumably this check comes from https://dom.spec.whatwg.org/#concept-range-bp-set? I'm a bit concerned that we're adding these extra checks in various places in the code; is our implementation missing them elsewhere, or is this a spec inconsistency, or are we working around tests that assume a certain order or checks that doesn't match what the spec says?
This comment has been minimized.
This comment has been minimized.
pshaughn
Feb 11, 2020
Author
Member
Tests, which Firefox and Chrome both pass in FYI, are assuming a certain order that doesn't match what the spec says.
This comment has been minimized.
This comment has been minimized.
jdm
added
S-awaiting-answer
S-needs-code-changes
and removed
S-awaiting-review
S-tests-failed
labels
|
@bors-servo r+ |
|
|
Selection DOM interface (but not actual UI selections) This is work towards #7492. I put new tests in the mozilla-specific directory rather than the main tree because I'm not sure how upstreamable they are; I'd like opinions on that point. This adds a new exposed interface, which I understand from tests/mozilla/interfaces.html is something requiring specific approval from someone allowed to give that approval. Things that aren't done here: - The spec doesn't describe what selection/script-and-style-elements.html wants, and what it wants seems to require some sort of layout query. - Actual UI interactions are not present at all; selection.rs has a few TODOs saying which methods I believe the eventual UI code should call for what actions, but there are a lot of fine points here that I don't know about. - I haven't touched Node; you can ask if a node's in the Selection's Range the same way you'd ask about any other Range, but there's not a faster path just for selection layout.
|
|
|
@bors-servo retry |
Selection DOM interface (but not actual UI selections) This is work towards #7492. I put new tests in the mozilla-specific directory rather than the main tree because I'm not sure how upstreamable they are; I'd like opinions on that point. This adds a new exposed interface, which I understand from tests/mozilla/interfaces.html is something requiring specific approval from someone allowed to give that approval. Things that aren't done here: - The spec doesn't describe what selection/script-and-style-elements.html wants, and what it wants seems to require some sort of layout query. - Actual UI interactions are not present at all; selection.rs has a few TODOs saying which methods I believe the eventual UI code should call for what actions, but there are a lot of fine points here that I don't know about. - I haven't touched Node; you can ask if a node's in the Selection's Range the same way you'd ask about any other Range, but there's not a faster path just for selection layout.
|
|
|
@bors-servo retry |
Selection DOM interface (but not actual UI selections) This is work towards #7492. I put new tests in the mozilla-specific directory rather than the main tree because I'm not sure how upstreamable they are; I'd like opinions on that point. This adds a new exposed interface, which I understand from tests/mozilla/interfaces.html is something requiring specific approval from someone allowed to give that approval. Things that aren't done here: - The spec doesn't describe what selection/script-and-style-elements.html wants, and what it wants seems to require some sort of layout query. - Actual UI interactions are not present at all; selection.rs has a few TODOs saying which methods I believe the eventual UI code should call for what actions, but there are a lot of fine points here that I don't know about. - I haven't touched Node; you can ask if a node's in the Selection's Range the same way you'd ask about any other Range, but there's not a faster path just for selection layout.
|
|
pshaughn commentedFeb 2, 2020
•
edited
This is work towards #7492.
I put new tests in the mozilla-specific directory rather than the main tree because I'm not sure how upstreamable they are; I'd like opinions on that point.
This adds a new exposed interface, which I understand from tests/mozilla/interfaces.html is something requiring specific approval from someone allowed to give that approval.
Things that aren't done here: