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

Added current_line variable to track current line #14661

Closed
wants to merge 6 commits into from

Moved comment and added function to match ElementCreator enum variant.

  • Loading branch information
karenher committed Dec 25, 2016
commit d80d24be413192e2374afc5d5881bf75210e45ed
@@ -147,6 +147,15 @@ pub enum ElementCreator {
ScriptCreated,
}

impl ElementCreator {
pub fn is_parser_created(&self) -> u64 {

This comment has been minimized.

Copy link
@jdm

jdm Dec 25, 2016

Member

The name of this method does not match what this method is actually doing. We should be returning a boolean here; the comment I made about extracting the line number belongs in a separate method.

match *self {
ElementCreator::ParserCreated(l) => l,
ElementCreator::ScriptCreated => 1,
}
}
}

pub enum AdjacentPosition {
BeforeBegin,
AfterEnd,
@@ -338,13 +338,13 @@ impl GlobalScope {
/// Evaluate JS code on this global scope.
pub fn evaluate_js_on_global_with_result(
&self, code: &str, rval: MutableHandleValue) {
self.evaluate_script_on_global_with_result(code, "", rval)
self.evaluate_script_on_global_with_result(code, "", rval, 1)
}

/// Evaluate a JS script on this global scope.
#[allow(unsafe_code)]
pub fn evaluate_script_on_global_with_result(
&self, code: &str, filename: &str, rval: MutableHandleValue) {
&self, code: &str, filename: &str, rval: MutableHandleValue, line_number: u32) {
let metadata = time::TimerMetadata {
url: if filename.is_empty() {
self.get_url().as_str().into()
@@ -365,7 +365,7 @@ impl GlobalScope {
let filename = CString::new(filename).unwrap();

let _ac = JSAutoCompartment::new(cx, globalhandle.get());
let options = CompileOptionsWrapper::new(cx, filename.as_ptr(), 1);
let options = CompileOptionsWrapper::new(cx, filename.as_ptr(), line_number);
unsafe {
if !Evaluate2(cx, options.ptr, code.as_ptr(),
code.len() as libc::size_t,
@@ -64,6 +64,9 @@ pub struct HTMLScriptElement {

/// The source this script was loaded from
load: DOMRefCell<Option<Result<ScriptOrigin, NetworkError>>>,

/// Track line line_number
line_number: u64,
}

impl HTMLScriptElement {
@@ -78,6 +81,7 @@ impl HTMLScriptElement {
ready_to_be_parser_executed: Cell::new(false),

This comment has been minimized.

Copy link
@jdm

jdm Dec 25, 2016

Member

The checks for ParserCeated on the previous line (and elsewhere in this PR) are still incorrect.

parser_document: JS::from_ref(document),
load: DOMRefCell::new(None),
line_number: creator.is_parser_created(),
}
}

@@ -495,7 +499,7 @@ impl HTMLScriptElement {
let window = window_from_node(self);
rooted!(in(window.get_cx()) let mut rval = UndefinedValue());
window.upcast::<GlobalScope>().evaluate_script_on_global_with_result(
&script.text, script.url.as_str(), rval.handle_mut());
&script.text, script.url.as_str(), rval.handle_mut(), 1);

This comment has been minimized.

Copy link
@jdm

jdm Dec 28, 2016

Member

This still needs to use self.line_number instead of being hardcoded.


// Step 6.
document.set_current_script(old_script.r());
@@ -100,7 +100,6 @@ struct Sink {
script: MutNullableJS<HTMLScriptElement>,
}

//TODO: Add ability to track lines to API of xml5ever
impl<'a> TreeSink for Sink {
type Handle = JS<Node>;

@@ -129,6 +128,7 @@ impl<'a> TreeSink for Sink {
ns: name.namespace_url,
local: name.local,
};
//TODO: Add ability to track lines to API of xml5ever
let elem = Element::create(name, prefix, &*self.document,
ElementCreator::ParserCreated(1));

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.