Skip to content

/ servo

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

Jump to bottom

Implement Element.client{Top,Left,Width,Height} #1713

Closed
jdm opened this issue Feb 20, 2014 · 16 comments
Closed

Implement Element.client{Top,Left,Width,Height} #1713

jdm opened this issue Feb 20, 2014 · 16 comments
Labels
A-content/dom C-is-this-done

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 20, 2014

http://dev.w3.org/csswg/cssom-view/#extensions-to-the-element-interface

Looking into the scrollTop and scrollLeft getters could be fun as well. Not sure what work would be involved with the setters, though.
@jdm jdm added the A-content/dom label Feb 20, 2014
@lpy
Copy link
Contributor

@lpy lpy commented Feb 20, 2014

I will work on this one.
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

I have a question about the get_css_select_results.

The get_css_select_results I implement in Node

    #[inline]
    pub fn get_css_select_results(&self) -> Arc<ComputedValues> {
        unsafe {
            let layout_data_ref = self.layout_data.data_cell.borrow();
            let shared_style_ref: *SharedLayoutData = cast::transmute(&layout_data_ref.get().get_ref().data);
            (*shared_style_ref).style.as_ref().unwrap().clone()
        }
    }

The SharedLayoutData I implement in style.

use extra::arc::Arc;
use properties::ComputedValues;
pub struct SharedLayoutData {
    /// The results of CSS styling for this node.
    style: Option<Arc<ComputedValues>>,
}
impl SharedLayoutData {
    pub fn new() -> SharedLayoutData {
        SharedLayoutData {
            style: None,
        }
    }
}

Then I change the style in PrivateLayoutData to shared_style: SharedLayoutData

The problem is that the get_ref() in get_css_select_results will return None.

The test code is

<html>
  <div id="div" style="width:100px; height:100px; padding:10px; border:5px solid;"></div>
  <script>
    var div = document.getElementById("div");
    alert(div);
    alert(div.clientTop);
  </script>
</html>

I am still trying to figure it out. Any idea? Thank you.
Please see my patch here
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

By the way, I did have one time that the code work.
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

Also, the alert(div.style) returns undefined.
@jdm
Copy link
Member Author

@jdm jdm commented Mar 9, 2014

The problem is that the script code is racing with layout, which may not have finished yet. At minimum you'll need to call Node::wait_until_safe_to_modify_dom, which will block until any pending layout operations are complete.
@jdm
Copy link
Member Author

@jdm jdm commented Mar 9, 2014

The unimplemented style property is tracked in #1721.
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

It didn't work to me.
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

Error message:

task 'ScriptTask' failed at 'called `Option::unwrap()` on a `None` value', /Users/lpy/moz/servo/src/compiler/rust/src/libstd/option.rs:133
failed at 'assertion failed: self.live_allocs.is_null()', /Users/lpy/moz/servo/src/compiler/rust/src/libstd/rt/local_heap.rs:135
[1]    83669 illegal hardware instruction  ./servo ../../test.html
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

I called wait_until_safe_to_modify_dom before the unsafe block in get_css_select_results.
@jdm
Copy link
Member Author

@jdm jdm commented Mar 9, 2014

Can you push your code to a branch?
@lpy
Copy link
Contributor

@lpy lpy commented Mar 9, 2014

Oh this one https://github.com/lpy/servo/commit/7bcf245f3f84c9efb2b41e8045eb45969c3533b9
@lpy
Copy link
Contributor

@lpy lpy commented Mar 11, 2014

@pcwalton @metajack @kmcallister Hi! Do you know how to extract the layout information from the layout task in the script? The query_layout can't return the things I want. I want display, content, border, padding, etc.
@jdm
Copy link
Member Author

@jdm jdm commented Mar 11, 2014

@lpy You can create a new message to send via query_layout and calculate whatever you need. You don't have to rely on existing message types.
@jdm jdm added the C-assigned label Apr 1, 2014
@jdm
Copy link
Member Author

@jdm jdm commented Apr 1, 2014

Are you still working on this, @lpy?
@jdm jdm removed the A-content/dom label Oct 23, 2014
@jdm jdm removed the C-assigned label Dec 17, 2014
@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jul 28, 2015

Was this completed in #6797?

/cc @glennw
@jdm
Copy link
Member Author

@jdm jdm commented Jul 28, 2015

Yep!
@jdm jdm closed this Jul 28, 2015
@peterjoel peterjoel mentioned this issue Feb 15, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom C-is-this-done
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@jdm @frewsxcv @lpy
You can’t perform that action at this time.