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

Make offset parent queries less buggy. #14839

Merged
merged 22 commits into from Jan 18, 2017
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
014ad76
Added assertion to process_offset_parent_query
Permutatrix Aug 26, 2016
0a5218d
Fixed ParentOffsetBorderBoxIterator's keeping track of parents
Permutatrix Aug 30, 2016
6623583
offsetParent queries now basically work on inline nodes.
Permutatrix Sep 27, 2016
18793af
Don't crash when offset parent querying an element not in the document
Permutatrix Jan 4, 2017
a61a263
Added tests/wpt/web-platform-tests/cssom-view/offsetPropertiesInline.…
Permutatrix Jan 4, 2017
6a76525
Replaced trivial pattern matching with is_none() and is_some()
Permutatrix Jan 4, 2017
fa0cb7c
Assert self.node_offset_box is None if fragment.node == self.node_add…
Permutatrix Jan 4, 2017
699fc4b
Use InlineFragmentNodeFlags in ParentOffsetBorderBoxIterator
Permutatrix Jan 5, 2017
923ecba
Updated expectations for passing tests
Permutatrix Jan 5, 2017
db5da15
Updated expectations for failing tests that were passing erroneously
Permutatrix Jan 5, 2017
4825169
Split 123-character line
Permutatrix Jan 5, 2017
defa7d9
Actually, node_position probably won't be needed later.
Permutatrix Jan 5, 2017
b75dcc0
Made assertions more helpful
Permutatrix Jan 6, 2017
5be1879
"above the root node" -> "below the root node"
Permutatrix Jan 6, 2017
290ebab
Moved and adjusted offset_properties_inline test
Permutatrix Jan 6, 2017
5ef7a0a
Don't store the offset parent's dimensions
Permutatrix Jan 7, 2017
686d2f8
Don't use rposition() and unwrap() to find parent info
Permutatrix Jan 7, 2017
74e34d6
Handle hypothetical fragments
Permutatrix Jan 13, 2017
cb0d43a
Broke up some comment lines that were a bit too long
Permutatrix Jan 13, 2017
af633b2
Use parent padding box, not border box
Permutatrix Jan 13, 2017
2cd5384
Don't take measurements from non-primary fragments
Permutatrix Jan 13, 2017
400a267
/css-text-3_dev/html/word-break-normal-zh-000.htm fails on Mac
Permutatrix Jan 18, 2017
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Replaced trivial pattern matching with is_none() and is_some()

  • Loading branch information
Permutatrix committed Jan 13, 2017
commit 6a7652510793078daf93d2b405da02c68e3ef88e
@@ -539,13 +539,11 @@ impl FragmentBorderBoxIterator for UnioningFragmentScrollAreaIterator {
// https://drafts.csswg.org/cssom-view/#extensions-to-the-htmlelement-interface
impl FragmentBorderBoxIterator for ParentOffsetBorderBoxIterator {
fn process(&mut self, fragment: &Fragment, level: i32, border_box: &Rect<Au>) {
match self.node_offset_box {
Some(_) => { }, // We've already found the node, so we already have its parent.
None => {
// Remove all nodes at this level or higher, as they can't be parents of this node.
self.parent_nodes.truncate(level as usize);
assert!(self.parent_nodes.len() == level as usize);
},
if self.node_offset_box.is_none() {
// We haven't found the node yet, so we're still looking for its parent.
// Remove all nodes at this level or higher, as they can't be parents of this node.
self.parent_nodes.truncate(level as usize);
assert!(self.parent_nodes.len() == level as usize);

This comment has been minimized.

@emilio

emilio Jan 5, 2017

Member

nit: use assert_eq!, and ideally a helpful assertion message.

}

if fragment.node == self.node_address {
@@ -581,7 +579,7 @@ impl FragmentBorderBoxIterator for ParentOffsetBorderBoxIterator {
// TODO: `position: fixed` on inline elements doesn't seem to work
// as of Sep 25, 2016, so I don't know how one will check if an
// inline element has it when it does.

This comment has been minimized.

@stshine

stshine Jan 6, 2017

Contributor

An element with position: absolute or position: fixed is guaranteed to be a block so you don't need to check that.

This comment has been minimized.

@notriddle

notriddle Jan 6, 2017

Contributor

Actually, it's a bit more complicated than that. The offset* JavaScript functions need to operate on the real boxes, rather than the hypothetical boxes.

In the case of Servo's implementation, the hypothetical absolute-inline box is a Fragment, while the real absolute-inline box is a BlockFlow.

} else if let Some(_) = self.node_offset_box {
} else if self.node_offset_box.is_some() {
// We've been processing fragments within the node, but now
// we've found one outside it. That means we're done.
// Hopefully.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.