Auto merge of #16394 - mckaymatt:line_num_offset__issue_15693, r=SimonSapin

Support line number offsets for inline stylesheets

<!-- Please describe your changes on the following line: -->
This allows accurate line numbers when reporting stylesheet errors.

@jdm This is going to require some effort to merge my changes with other recent changes to `ParserContext`. Because of that I would appreciate a quick sanity check before I put the time into performing the merge.
For example, should I store the `offset` as a u64, or should it be an Option?

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #15693 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16394)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-04-13 20:12:08 -05:00 committed by GitHub
commit 94397ff0b4
11 changed files with 87 additions and 37 deletions

View file

@ -40,6 +40,7 @@ pub struct HTMLStyleElement {
in_stack_of_open_elements: Cell<bool>,
pending_loads: Cell<u32>,
any_failed_load: Cell<bool>,
line_number: u64,
}
impl HTMLStyleElement {
@ -55,6 +56,7 @@ impl HTMLStyleElement {
in_stack_of_open_elements: Cell::new(creator.is_parser_created()),
pending_loads: Cell::new(0),
any_failed_load: Cell::new(false),
line_number: creator.return_line_number(),
}
}
@ -92,7 +94,8 @@ impl HTMLStyleElement {
let loader = StylesheetLoader::for_element(self.upcast());
let sheet = Stylesheet::from_str(&data, win.get_url(), Origin::Author, mq,
shared_lock, Some(&loader),
win.css_error_reporter());
win.css_error_reporter(),
self.line_number);
let sheet = Arc::new(sheet);

View file

@ -26,12 +26,14 @@ impl ParseErrorReporter for CSSErrorReporter {
input: &mut Parser,
position: SourcePosition,
message: &str,
url: &ServoUrl) {
url: &ServoUrl,
line_number_offset: u64) {
let location = input.source_location(position);
let line_offset = location.line + line_number_offset as usize;
if log_enabled!(log::LogLevel::Info) {
info!("Url:\t{}\n{}:{} {}",
url.as_str(),
location.line,
line_offset,
location.column,
message)
}

View file

@ -66,7 +66,8 @@ impl Stylesheet {
Arc::new(shared_lock.wrap(media)),
shared_lock,
stylesheet_loader,
error_reporter)
error_reporter,
0u64)
}
/// Updates an empty stylesheet with a set of bytes that reached over the

View file

@ -12,7 +12,7 @@ use stylesheets::UrlExtraData;
/// A generic trait for an error reporter.
pub trait ParseErrorReporter : Sync + Send {
/// Called the style engine detects an error.
/// Called when the style engine detects an error.
///
/// Returns the current input being parsed, the source position it was
/// reported from, and a message.
@ -20,7 +20,8 @@ pub trait ParseErrorReporter : Sync + Send {
input: &mut Parser,
position: SourcePosition,
message: &str,
url: &UrlExtraData);
url: &UrlExtraData,
line_number_offset: u64);
}
/// An error reporter that reports the errors to the `info` log channel.
@ -32,10 +33,12 @@ impl ParseErrorReporter for StdoutErrorReporter {
input: &mut Parser,
position: SourcePosition,
message: &str,
url: &UrlExtraData) {
url: &UrlExtraData,
line_number_offset: u64) {
if log_enabled!(log::LogLevel::Info) {
let location = input.source_location(position);
info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, message)
let line_offset = location.line + line_number_offset as usize;
info!("Url:\t{}\n{}:{} {}", url.as_str(), line_offset, location.column, message)
}
}
}

View file

@ -22,6 +22,8 @@ pub struct ParserContext<'a> {
pub error_reporter: &'a ParseErrorReporter,
/// The current rule type, if any.
pub rule_type: Option<CssRuleType>,
/// line number offsets for inline stylesheets
pub line_number_offset: u64,
}
impl<'a> ParserContext<'a> {
@ -36,6 +38,7 @@ impl<'a> ParserContext<'a> {
url_data: url_data,
error_reporter: error_reporter,
rule_type: rule_type,
line_number_offset: 0u64,
}
}
@ -51,16 +54,34 @@ impl<'a> ParserContext<'a> {
pub fn new_with_rule_type(context: &'a ParserContext,
rule_type: Option<CssRuleType>)
-> ParserContext<'a> {
Self::new(context.stylesheet_origin,
context.url_data,
context.error_reporter,
rule_type)
ParserContext {
stylesheet_origin: context.stylesheet_origin,
url_data: context.url_data,
error_reporter: context.error_reporter,
rule_type: rule_type,
line_number_offset: context.line_number_offset,
}
}
/// Get the rule type, which assumes that one is available.
pub fn rule_type(&self) -> CssRuleType {
self.rule_type.expect("Rule type expected, but none was found.")
}
/// Create a parser context for inline CSS which accepts additional line offset argument.
pub fn new_with_line_number_offset(stylesheet_origin: Origin,
url_data: &'a UrlExtraData,
error_reporter: &'a ParseErrorReporter,
line_number_offset: u64)
-> ParserContext<'a> {
ParserContext {
stylesheet_origin: stylesheet_origin,
url_data: url_data,
error_reporter: error_reporter,
rule_type: None,
line_number_offset: line_number_offset,
}
}
}
/// Defaults to a no-op.
@ -71,7 +92,10 @@ pub fn log_css_error(input: &mut Parser,
message: &str,
parsercontext: &ParserContext) {
let url_data = parsercontext.url_data;
parsercontext.error_reporter.report_error(input, position, message, url_data);
let line_number_offset = parsercontext.line_number_offset;
parsercontext.error_reporter.report_error(input, position,
message, url_data,
line_number_offset);
}
// XXXManishearth Replace all specified value parse impls with impls of this

View file

@ -328,7 +328,8 @@ impl ParseErrorReporter for MemoryHoleReporter {
_: &mut Parser,
_: SourcePosition,
_: &str,
_: &UrlExtraData) {
_: &UrlExtraData,
_: u64) {
// do nothing
}
}
@ -666,7 +667,7 @@ impl Stylesheet {
let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules(
css, url_data, existing.origin, &mut namespaces,
&existing.shared_lock, stylesheet_loader, error_reporter,
);
0u64);
*existing.namespaces.write() = namespaces;
existing.dirty_on_viewport_size_change
@ -683,7 +684,8 @@ impl Stylesheet {
namespaces: &mut Namespaces,
shared_lock: &SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter)
error_reporter: &ParseErrorReporter,
line_number_offset: u64)
-> (Vec<CssRule>, bool) {
let mut rules = Vec::new();
let mut input = Parser::new(css);
@ -692,7 +694,8 @@ impl Stylesheet {
namespaces: namespaces,
shared_lock: shared_lock,
loader: stylesheet_loader,
context: ParserContext::new(origin, url_data, error_reporter, None),
context: ParserContext::new_with_line_number_offset(origin, url_data, error_reporter,
line_number_offset),
state: Cell::new(State::Start),
};
@ -726,11 +729,12 @@ impl Stylesheet {
media: Arc<Locked<MediaList>>,
shared_lock: SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter) -> Stylesheet {
error_reporter: &ParseErrorReporter,
line_number_offset: u64) -> Stylesheet {
let mut namespaces = Namespaces::default();
let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules(
css, &url_data, origin, &mut namespaces,
&shared_lock, stylesheet_loader, error_reporter,
&shared_lock, stylesheet_loader, error_reporter, line_number_offset
);
Stylesheet {
origin: origin,