From 393ab5ab1e9fa4a26dfdb985af1036eb199d9ea3 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 24 Nov 2017 11:28:34 +0800 Subject: [PATCH 1/5] geckolib: Return from Servo_ComputeColor whether the value was currentcolor. --- ports/geckolib/glue.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 4ddb28f4f94..94c3ea6efcb 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -4577,6 +4577,7 @@ pub extern "C" fn Servo_ComputeColor( current_color: structs::nscolor, value: *const nsAString, result_color: *mut structs::nscolor, + was_current_color: *mut bool, ) -> bool { use style::gecko; @@ -4611,6 +4612,11 @@ pub extern "C" fn Servo_ComputeColor( Some(computed_color) => { let rgba = computed_color.to_rgba(current_color); *result_color = gecko::values::convert_rgba_to_nscolor(&rgba); + if !was_current_color.is_null() { + unsafe { + *was_current_color = computed_color.is_currentcolor(); + } + } true } None => false, From 7940061c9a76ce41acf13b227233860b983642c9 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 4 Dec 2017 12:49:16 +0800 Subject: [PATCH 2/5] geckolib: Allow an ErrorReporter to be created with null URL data. This allows us to create an ErrorReporter that will report errors as being "from DOM" rather than a specific URL. Also, factor out a method on ErrorReporter so that we don't need to pass in a UrlExtraData reference. --- ports/geckolib/error_reporter.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/ports/geckolib/error_reporter.rs b/ports/geckolib/error_reporter.rs index b39a7c69515..efca5b55ede 100644 --- a/ports/geckolib/error_reporter.rs +++ b/ports/geckolib/error_reporter.rs @@ -16,7 +16,6 @@ use style::gecko_bindings::bindings::Gecko_ReportUnexpectedCSSError; use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI}; use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter; use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData; -use style::gecko_bindings::sugar::refptr::RefPtr; use style::stylesheets::UrlExtraData; use style_traits::StyleParseErrorKind; @@ -29,10 +28,12 @@ impl ErrorReporter { /// Create a new instance of the Gecko error reporter. pub fn new(sheet: *mut ServoStyleSheet, loader: *mut Loader, - url: *mut RawUrlExtraData) -> ErrorReporter { + extra_data: *mut RawUrlExtraData) -> ErrorReporter { unsafe { - let url = RefPtr::from_ptr_ref(&url); - ErrorReporter(Gecko_CreateCSSErrorReporter(sheet, loader, url.mBaseURI.raw::())) + let url = extra_data.as_ref() + .map(|d| d.mBaseURI.raw::()) + .unwrap_or(ptr::null_mut()); + ErrorReporter(Gecko_CreateCSSErrorReporter(sheet, loader, url)) } } } @@ -364,11 +365,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> { } } -impl ParseErrorReporter for ErrorReporter { - fn report_error(&self, - _url: &UrlExtraData, - location: SourceLocation, - error: ContextualParseError) { +impl ErrorReporter { + pub fn report(&self, location: SourceLocation, error: ContextualParseError) { let (pre, name, action) = error.to_gecko_message(); let suffix = match action { Action::Nothing => ptr::null(), @@ -400,3 +398,14 @@ impl ParseErrorReporter for ErrorReporter { } } } + +impl ParseErrorReporter for ErrorReporter { + fn report_error( + &self, + _url: &UrlExtraData, + location: SourceLocation, + error: ContextualParseError + ) { + self.report(location, error) + } +} From 7015d5b22e8157450677cf0e01d83e8afc8c0d1d Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 4 Dec 2017 12:52:39 +0800 Subject: [PATCH 3/5] style: Add a new CSS error type for lone value parsing errors. This allows us to report errors in functions that want to just parse a single CSS value, rather than a value for a particular property declaration. The one value type that we need to support for now is value parsing errors, so just add formatting/support for that. --- components/style/error_reporting.rs | 5 +++++ ports/geckolib/error_reporter.rs | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/components/style/error_reporting.rs b/components/style/error_reporting.rs index 19e89e2338f..d989857d141 100644 --- a/components/style/error_reporting.rs +++ b/components/style/error_reporting.rs @@ -46,6 +46,8 @@ pub enum ContextualParseError<'a> { InvalidCounterStyleExtendsWithAdditiveSymbols, /// A media rule was invalid for some reason. InvalidMediaRule(&'a str, ParseError<'a>), + /// A value was not recognized. + UnsupportedValue(&'a str, ParseError<'a>), } impl<'a> fmt::Display for ContextualParseError<'a> { @@ -173,6 +175,9 @@ impl<'a> fmt::Display for ContextualParseError<'a> { write!(f, "Invalid media rule: {}, ", media_rule)?; parse_error_to_str(err, f) } + ContextualParseError::UnsupportedValue(_value, ref err) => { + parse_error_to_str(err, f) + } } } } diff --git a/ports/geckolib/error_reporter.rs b/ports/geckolib/error_reporter.rs index efca5b55ede..e3e3cdf49c3 100644 --- a/ports/geckolib/error_reporter.rs +++ b/ports/geckolib/error_reporter.rs @@ -17,7 +17,7 @@ use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI}; use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter; use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData; use style::stylesheets::UrlExtraData; -use style_traits::StyleParseErrorKind; +use style_traits::{StyleParseErrorKind, ValueParseErrorKind}; pub type ErrorKind<'i> = ParseErrorKind<'i, StyleParseErrorKind<'i>>; @@ -139,6 +139,9 @@ fn extract_error_params<'a>(err: ErrorKind<'a>) -> Option> { ParseErrorKind::Custom( StyleParseErrorKind::ExpectedIdentifier(token) + ) | + ParseErrorKind::Custom( + StyleParseErrorKind::ValueError(ValueParseErrorKind::InvalidColor(token)) ) => { (Some(ErrorString::UnexpectedToken(token)), None) } @@ -198,7 +201,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> { ContextualParseError::UnsupportedRule(s, err) | ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) | ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) | - ContextualParseError::InvalidMediaRule(s, err) => { + ContextualParseError::InvalidMediaRule(s, err) | + ContextualParseError::UnsupportedValue(s, err) => { (s.into(), err.kind) } ContextualParseError::InvalidCounterStyleWithoutSymbols(s) | @@ -360,6 +364,23 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> { ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) | ContextualParseError::InvalidFontFeatureValuesRule(..) => (b"PEUnknownAtRule\0", Action::Skip), + ContextualParseError::UnsupportedValue(_, ParseError { ref kind, .. }) => { + match *kind { + ParseErrorKind::Custom( + StyleParseErrorKind::ValueError( + ValueParseErrorKind::InvalidColor(..) + ) + ) => (b"PEColorNotColor", Action::Nothing), + _ => { + // Not the best error message, since we weren't parsing + // a declaration, just a value. But we don't produce + // UnsupportedValue errors other than InvalidColors + // currently. + debug_assert!(false, "should use a more specific error message"); + (b"PEDeclDropped", Action::Nothing) + } + } + } }; (None, msg, action) } From 9a31d0d8d82ae907f29bdbfa131da7197472b5e9 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 4 Dec 2017 12:53:13 +0800 Subject: [PATCH 4/5] geckolib: Allow Servo_ComputeColor to report errors to the console. --- ports/geckolib/glue.rs | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 94c3ea6efcb..a4c12642623 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use cssparser::{Parser, ParserInput}; +use cssparser::{ParseErrorKind, Parser, ParserInput}; use cssparser::ToCss as ParserToCss; use env_logger::LogBuilder; use malloc_size_of::MallocSizeOfOps; @@ -23,7 +23,7 @@ use style::data::{ElementStyles, self}; use style::dom::{ShowSubtreeData, TDocument, TElement, TNode}; use style::driver; use style::element_state::{DocumentState, ElementState}; -use style::error_reporting::{NullReporter, ParseErrorReporter}; +use style::error_reporting::{ContextualParseError, NullReporter, ParseErrorReporter}; use style::font_metrics::{FontMetricsProvider, get_metrics_provider_for_product}; use style::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl}; use style::gecko::global_style_data::{GLOBAL_STYLE_DATA, GlobalStyleData, STYLE_THREAD_POOL}; @@ -153,7 +153,7 @@ use style::values::distance::ComputeSquaredDistance; use style::values::specified; use style::values::specified::gecko::IntersectionObserverRootMargin; use style::values::specified::source_size_list::SourceSizeList; -use style_traits::{ParsingMode, ToCss}; +use style_traits::{ParsingMode, StyleParseErrorKind, ToCss}; use super::error_reporter::ErrorReporter; use super::stylesheet_loader::StylesheetLoader; @@ -4557,10 +4557,31 @@ pub unsafe extern "C" fn Servo_SelectorList_Drop(list: RawServoSelectorListOwned let _ = list.into_box::<::selectors::SelectorList>(); } -fn parse_color(value: &str) -> Result { +fn parse_color( + value: &str, + error_reporter: Option<&ErrorReporter>, +) -> Result { let mut input = ParserInput::new(value); let mut parser = Parser::new(&mut input); - parser.parse_entirely(specified::Color::parse_color).map_err(|_| ()) + let start_position = parser.position(); + parser.parse_entirely(specified::Color::parse_color).map_err(|err| { + if let Some(error_reporter) = error_reporter { + match err.kind { + ParseErrorKind::Custom(StyleParseErrorKind::ValueError(..)) => { + let location = err.location.clone(); + let error = ContextualParseError::UnsupportedValue( + parser.slice_from(start_position), + err, + ); + error_reporter.report(location, error); + } + // Ignore other kinds of errors that might be reported, such as + // ParseErrorKind::Basic(BasicParseErrorKind::UnexpectedToken), + // since Gecko doesn't report those to the error console. + _ => {} + } + } + }) } #[no_mangle] @@ -4568,7 +4589,7 @@ pub extern "C" fn Servo_IsValidCSSColor( value: *const nsAString, ) -> bool { let value = unsafe { (*value).to_string() }; - parse_color(&value).is_ok() + parse_color(&value, None).is_ok() } #[no_mangle] @@ -4578,6 +4599,7 @@ pub extern "C" fn Servo_ComputeColor( value: *const nsAString, result_color: *mut structs::nscolor, was_current_color: *mut bool, + loader: *mut Loader, ) -> bool { use style::gecko; @@ -4585,7 +4607,12 @@ pub extern "C" fn Servo_ComputeColor( let value = unsafe { (*value).to_string() }; let result_color = unsafe { result_color.as_mut().unwrap() }; - match parse_color(&value) { + let reporter = unsafe { loader.as_mut() }.map(|loader| { + // Make an ErrorReporter that will report errors as being "from DOM". + ErrorReporter::new(ptr::null_mut(), loader, ptr::null_mut()) + }); + + match parse_color(&value, reporter.as_ref()) { Ok(specified_color) => { let computed_color = match raw_data { Some(raw_data) => { From 64c5a8a6e8d23687d99a2e1080239340c3976227 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 4 Dec 2017 13:42:17 +0800 Subject: [PATCH 5/5] style: Regenerate Gecko bindings. --- components/style/gecko/generated/bindings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 93dba482c0d..b7757bc3f31 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -1577,7 +1577,7 @@ extern "C" { } extern "C" { pub fn Servo_IsValidCSSColor ( value : * const nsAString , ) -> bool ; } extern "C" { - pub fn Servo_ComputeColor ( set : RawServoStyleSetBorrowedOrNull , current_color : nscolor , value : * const nsAString , result_color : * mut nscolor , ) -> bool ; + pub fn Servo_ComputeColor ( set : RawServoStyleSetBorrowedOrNull , current_color : nscolor , value : * const nsAString , result_color : * mut nscolor , was_current_color : * mut bool , loader : * mut Loader , ) -> bool ; } extern "C" { pub fn Servo_ParseIntersectionObserverRootMargin ( value : * const nsAString , result : * mut nsCSSRect , ) -> bool ; } extern "C" {