From 4913d9c900178cac9353b4fe7b6c596248f8f9a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 9 Feb 2018 17:07:42 +0100 Subject: [PATCH] style: Handle quirks-mode changes correctly. The reason why bug 1355721 regressed this is because in non-e10s we definitely flush before parsing the standards quirks-mode. And bug 1355721 introduced an unconditional UpdateStylistIfNeeded, unless the counter style / font equivalents. That means that the stylist wouldn't remain on its initial state after the first flush, which itself means that when the compat mode changed, the UA and user rules were already on the stylist with the quirks mode keys. That makes class-names be keyed in ascii lowercase. After that no user style changed, so no rebuild happens for the cascade data in the user origin, so we keep looking at the wrong keys indefinitely. We should try to avoid the flush there and ensure that by the time we create a pres shell the quirks mode is already up-to-date... Bug: 1394233 Reviewed-by: xidorn MozReview-Commit-ID: 25dD2bca3tN --- components/style/stylist.rs | 13 +++++++------ ports/geckolib/glue.rs | 15 ++++----------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 18e5fb4c7d1..56f7f4b2fea 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -77,6 +77,9 @@ impl UserAgentCascadeDataCache { } } + // FIXME(emilio): This may need to be keyed on quirks-mode too, though there + // aren't class / id selectors on those sheets, usually, so it's probably + // ok... fn lookup<'a, I, S>( &'a mut self, sheets: I, @@ -1146,13 +1149,11 @@ impl Stylist { /// Sets the quirks mode of the document. pub fn set_quirks_mode(&mut self, quirks_mode: QuirksMode) { - // FIXME(emilio): We don't seem to change the quirks mode dynamically - // during multiple layout passes, but this is totally bogus, in the - // sense that it's updated asynchronously. - // - // This should probably be an argument to `update`, and use the quirks - // mode info in the `SharedLayoutContext`. + if self.quirks_mode == quirks_mode { + return; + } self.quirks_mode = quirks_mode; + self.force_stylesheet_origins_dirty(OriginSet::all()); } /// Returns the applicable CSS declarations for the given element. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index ec88e40fe83..d5079bf293f 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2450,19 +2450,12 @@ pub extern "C" fn Servo_StyleSet_Drop(data: RawServoStyleSetOwned) { let _ = data.into_box::(); } - -/// Updating the stylesheets and redoing selector matching is always happens -/// before the document element is inserted. Therefore we don't need to call -/// `force_dirty` here. #[no_mangle] -pub extern "C" fn Servo_StyleSet_CompatModeChanged(raw_data: RawServoStyleSetBorrowed) { +pub unsafe extern "C" fn Servo_StyleSet_CompatModeChanged(raw_data: RawServoStyleSetBorrowed) { let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - let quirks_mode = unsafe { - (*data.stylist.device().pres_context().mDocument.raw::()) - .mCompatMode - }; - - data.stylist.set_quirks_mode(quirks_mode.into()); + let doc = + &*data.stylist.device().pres_context().mDocument.raw::(); + data.stylist.set_quirks_mode(QuirksMode::from(doc.mCompatMode)); } fn parse_property_into(