Support line number offsets for inline stylesheets

This commit is contained in:
mckaymatt 2017-04-12 19:33:03 -04:00
parent ea20bd6f63
commit 5d8cbd8e6a
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>, in_stack_of_open_elements: Cell<bool>,
pending_loads: Cell<u32>, pending_loads: Cell<u32>,
any_failed_load: Cell<bool>, any_failed_load: Cell<bool>,
line_number: u64,
} }
impl HTMLStyleElement { impl HTMLStyleElement {
@ -55,6 +56,7 @@ impl HTMLStyleElement {
in_stack_of_open_elements: Cell::new(creator.is_parser_created()), in_stack_of_open_elements: Cell::new(creator.is_parser_created()),
pending_loads: Cell::new(0), pending_loads: Cell::new(0),
any_failed_load: Cell::new(false), 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 loader = StylesheetLoader::for_element(self.upcast());
let sheet = Stylesheet::from_str(&data, win.get_url(), Origin::Author, mq, let sheet = Stylesheet::from_str(&data, win.get_url(), Origin::Author, mq,
shared_lock, Some(&loader), shared_lock, Some(&loader),
win.css_error_reporter()); win.css_error_reporter(),
self.line_number);
let sheet = Arc::new(sheet); let sheet = Arc::new(sheet);

View file

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

View file

@ -66,7 +66,8 @@ impl Stylesheet {
Arc::new(shared_lock.wrap(media)), Arc::new(shared_lock.wrap(media)),
shared_lock, shared_lock,
stylesheet_loader, stylesheet_loader,
error_reporter) error_reporter,
0u64)
} }
/// Updates an empty stylesheet with a set of bytes that reached over the /// 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. /// A generic trait for an error reporter.
pub trait ParseErrorReporter : Sync + Send { 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 /// Returns the current input being parsed, the source position it was
/// reported from, and a message. /// reported from, and a message.
@ -20,7 +20,8 @@ pub trait ParseErrorReporter : Sync + Send {
input: &mut Parser, input: &mut Parser,
position: SourcePosition, position: SourcePosition,
message: &str, message: &str,
url: &UrlExtraData); url: &UrlExtraData,
line_number_offset: u64);
} }
/// An error reporter that reports the errors to the `info` log channel. /// An error reporter that reports the errors to the `info` log channel.
@ -32,10 +33,12 @@ impl ParseErrorReporter for StdoutErrorReporter {
input: &mut Parser, input: &mut Parser,
position: SourcePosition, position: SourcePosition,
message: &str, message: &str,
url: &UrlExtraData) { url: &UrlExtraData,
line_number_offset: u64) {
if log_enabled!(log::LogLevel::Info) { if log_enabled!(log::LogLevel::Info) {
let location = input.source_location(position); 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, pub error_reporter: &'a ParseErrorReporter,
/// The current rule type, if any. /// The current rule type, if any.
pub rule_type: Option<CssRuleType>, pub rule_type: Option<CssRuleType>,
/// line number offsets for inline stylesheets
pub line_number_offset: u64,
} }
impl<'a> ParserContext<'a> { impl<'a> ParserContext<'a> {
@ -36,6 +38,7 @@ impl<'a> ParserContext<'a> {
url_data: url_data, url_data: url_data,
error_reporter: error_reporter, error_reporter: error_reporter,
rule_type: rule_type, 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, pub fn new_with_rule_type(context: &'a ParserContext,
rule_type: Option<CssRuleType>) rule_type: Option<CssRuleType>)
-> ParserContext<'a> { -> ParserContext<'a> {
Self::new(context.stylesheet_origin, ParserContext {
context.url_data, stylesheet_origin: context.stylesheet_origin,
context.error_reporter, url_data: context.url_data,
rule_type) 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. /// Get the rule type, which assumes that one is available.
pub fn rule_type(&self) -> CssRuleType { pub fn rule_type(&self) -> CssRuleType {
self.rule_type.expect("Rule type expected, but none was found.") 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. /// Defaults to a no-op.
@ -71,7 +92,10 @@ pub fn log_css_error(input: &mut Parser,
message: &str, message: &str,
parsercontext: &ParserContext) { parsercontext: &ParserContext) {
let url_data = parsercontext.url_data; 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 // XXXManishearth Replace all specified value parse impls with impls of this

View file

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

View file

@ -496,7 +496,7 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyl
Arc::new(Stylesheet::from_str( Arc::new(Stylesheet::from_str(
"", unsafe { dummy_url_data() }.clone(), origin, "", unsafe { dummy_url_data() }.clone(), origin,
Arc::new(shared_lock.wrap(MediaList::empty())), Arc::new(shared_lock.wrap(MediaList::empty())),
shared_lock, None, &StdoutErrorReporter) shared_lock, None, &StdoutErrorReporter, 0u64)
).into_strong() ).into_strong()
} }
@ -539,7 +539,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
Arc::new(Stylesheet::from_str( Arc::new(Stylesheet::from_str(
input, url_data.clone(), origin, media, input, url_data.clone(), origin, media,
shared_lock, loader, &StdoutErrorReporter) shared_lock, loader, &StdoutErrorReporter, 0u64)
).into_strong() ).into_strong()
} }

View file

@ -19,8 +19,12 @@ use style_traits::ToCss;
pub struct CSSErrorReporterTest; pub struct CSSErrorReporterTest;
impl ParseErrorReporter for CSSErrorReporterTest { impl ParseErrorReporter for CSSErrorReporterTest {
fn report_error(&self, _input: &mut Parser, _position: SourcePosition, _message: &str, fn report_error(&self,
_url: &ServoUrl) { _input: &mut Parser,
_position: SourcePosition,
_message: &str,
_url: &ServoUrl,
_line_number_offset: u64) {
} }
} }
@ -33,7 +37,7 @@ fn test_media_rule<F>(css: &str, callback: F)
let media_list = Arc::new(lock.wrap(MediaList::empty())); let media_list = Arc::new(lock.wrap(MediaList::empty()));
let stylesheet = Stylesheet::from_str( let stylesheet = Stylesheet::from_str(
css, url, Origin::Author, media_list, lock, css, url, Origin::Author, media_list, lock,
None, &CSSErrorReporterTest); None, &CSSErrorReporterTest, 0u64);
let mut rule_count = 0; let mut rule_count = 0;
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
media_queries(&guard, &stylesheet.rules.read_with(&guard).0, &mut |mq| { media_queries(&guard, &stylesheet.rules.read_with(&guard).0, &mut |mq| {
@ -62,7 +66,7 @@ fn media_query_test(device: &Device, css: &str, expected_rule_count: usize) {
let media_list = Arc::new(lock.wrap(MediaList::empty())); let media_list = Arc::new(lock.wrap(MediaList::empty()));
let ss = Stylesheet::from_str( let ss = Stylesheet::from_str(
css, url, Origin::Author, media_list, lock, css, url, Origin::Author, media_list, lock,
None, &CSSErrorReporterTest); None, &CSSErrorReporterTest, 0u64);
let mut rule_count = 0; let mut rule_count = 0;
ss.effective_style_rules(device, &ss.shared_lock.read(), |_| rule_count += 1); ss.effective_style_rules(device, &ss.shared_lock.read(), |_| rule_count += 1);
assert!(rule_count == expected_rule_count, css.to_owned()); assert!(rule_count == expected_rule_count, css.to_owned());

View file

@ -16,9 +16,15 @@ use test::{self, Bencher};
struct ErrorringErrorReporter; struct ErrorringErrorReporter;
impl ParseErrorReporter for ErrorringErrorReporter { impl ParseErrorReporter for ErrorringErrorReporter {
fn report_error(&self, _input: &mut Parser, position: SourcePosition, message: &str, fn report_error(&self,
url: &ServoUrl) { input: &mut Parser,
panic!("CSS error: {}\t\n{:?} {}", url.as_str(), position, message); position: SourcePosition,
message: &str,
url: &ServoUrl,
line_number_offset: u64) {
let location = input.source_location(position);
let line_offset = location.line + line_number_offset as usize;
panic!("CSS error: {}\t\n{}:{} {}", url.as_str(), line_offset, location.column, message);
} }
} }
@ -51,7 +57,8 @@ fn parse_rules(css: &str) -> Vec<(StyleSource, CascadeLevel)> {
media, media,
lock, lock,
None, None,
&ErrorringErrorReporter); &ErrorringErrorReporter,
0u64);
let guard = s.shared_lock.read(); let guard = s.shared_lock.read();
let rules = s.rules.read_with(&guard); let rules = s.rules.read_with(&guard);
rules.0.iter().filter_map(|rule| { rules.0.iter().filter_map(|rule| {

View file

@ -65,7 +65,7 @@ fn test_parse_stylesheet() {
let lock = SharedRwLock::new(); let lock = SharedRwLock::new();
let media = Arc::new(lock.wrap(MediaList::empty())); let media = Arc::new(lock.wrap(MediaList::empty()));
let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock, let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock,
None, &CSSErrorReporterTest); None, &CSSErrorReporterTest, 0u64);
let mut namespaces = Namespaces::default(); let mut namespaces = Namespaces::default();
namespaces.default = Some(ns!(html)); namespaces.default = Some(ns!(html));
let expected = Stylesheet { let expected = Stylesheet {
@ -293,16 +293,17 @@ impl ParseErrorReporter for CSSInvalidErrorReporterTest {
input: &mut CssParser, input: &mut CssParser,
position: SourcePosition, position: SourcePosition,
message: &str, message: &str,
url: &ServoUrl) { url: &ServoUrl,
line_number_offset: u64) {
let location = input.source_location(position); let location = input.source_location(position);
let line_offset = location.line + line_number_offset as usize;
let mut errors = self.errors.lock().unwrap(); let mut errors = self.errors.lock().unwrap();
errors.push( errors.push(
CSSError{ CSSError{
url: url.clone(), url: url.clone(),
line: location.line, line: line_offset,
column: location.column, column: location.column,
message: message.to_owned() message: message.to_owned()
} }
@ -328,18 +329,18 @@ fn test_report_error_stylesheet() {
let lock = SharedRwLock::new(); let lock = SharedRwLock::new();
let media = Arc::new(lock.wrap(MediaList::empty())); let media = Arc::new(lock.wrap(MediaList::empty()));
Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock, Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock,
None, &error_reporter); None, &error_reporter, 5u64);
let mut errors = errors.lock().unwrap(); let mut errors = errors.lock().unwrap();
let error = errors.pop().unwrap(); let error = errors.pop().unwrap();
assert_eq!("Unsupported property declaration: 'invalid: true;'", error.message); assert_eq!("Unsupported property declaration: 'invalid: true;'", error.message);
assert_eq!(5, error.line); assert_eq!(10, error.line);
assert_eq!(9, error.column); assert_eq!(9, error.column);
let error = errors.pop().unwrap(); let error = errors.pop().unwrap();
assert_eq!("Unsupported property declaration: 'display: invalid;'", error.message); assert_eq!("Unsupported property declaration: 'display: invalid;'", error.message);
assert_eq!(4, error.line); assert_eq!(9, error.line);
assert_eq!(9, error.column); assert_eq!(9, error.column);
// testing for the url // testing for the url

View file

@ -31,7 +31,8 @@ macro_rules! stylesheet {
Arc::new($shared_lock.wrap(MediaList::empty())), Arc::new($shared_lock.wrap(MediaList::empty())),
$shared_lock, $shared_lock,
None, None,
&$error_reporter &$error_reporter,
0u64
)) ))
} }
} }