diff --git a/components/gfx/font.rs b/components/gfx/font.rs index 22261f1fc29..17625dc7fe4 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -146,7 +146,8 @@ impl Font { } let mut glyphs = GlyphStore::new(text.chars().count(), - options.flags.contains(IS_WHITESPACE_SHAPING_FLAG)); + options.flags.contains(IS_WHITESPACE_SHAPING_FLAG), + options.flags.contains(RTL_FLAG)); shaper.as_ref().unwrap().shape_text(text, options, &mut glyphs); let glyphs = Arc::new(glyphs); diff --git a/components/gfx/text/glyph.rs b/components/gfx/text/glyph.rs index 435096486a8..75017144155 100644 --- a/components/gfx/text/glyph.rs +++ b/components/gfx/text/glyph.rs @@ -512,6 +512,7 @@ pub struct GlyphStore { detail_store: DetailedGlyphStore, is_whitespace: bool, + is_rtl: bool, } int_range_index! { @@ -526,13 +527,14 @@ impl<'a> GlyphStore { /// Initializes the glyph store, but doesn't actually shape anything. /// /// Use the `add_*` methods to store glyph data. - pub fn new(length: usize, is_whitespace: bool) -> GlyphStore { + pub fn new(length: usize, is_whitespace: bool, is_rtl: bool) -> GlyphStore { assert!(length > 0); GlyphStore { entry_buffer: vec![GlyphEntry::initial(); length], detail_store: DetailedGlyphStore::new(), is_whitespace: is_whitespace, + is_rtl: is_rtl, } } @@ -633,8 +635,8 @@ impl<'a> GlyphStore { GlyphIterator { store: self, - char_index: rang.begin(), - char_range: rang.each_index(), + char_index: if self.is_rtl { rang.end() } else { rang.begin() - CharIndex(1) }, + char_range: *rang, glyph_range: None, } } @@ -736,7 +738,7 @@ impl<'a> GlyphStore { pub struct GlyphIterator<'a> { store: &'a GlyphStore, char_index: CharIndex, - char_range: EachIndex, + char_range: Range, glyph_range: Option>, } @@ -776,23 +778,28 @@ impl<'a> Iterator for GlyphIterator<'a> { // `next_complex_glyph()`. #[inline(always)] fn next(&mut self) -> Option<(CharIndex, GlyphInfo<'a>)> { - // Would use 'match' here but it borrows contents in a way that - // interferes with mutation. + // Would use 'match' here but it borrows contents in a way that interferes with mutation. if self.glyph_range.is_some() { - self.next_glyph_range() + return self.next_glyph_range() + } + + // No glyph range. Look at next character. + self.char_index = self.char_index + if self.store.is_rtl { + CharIndex(-1) } else { - // No glyph range. Look at next character. - self.char_range.next().and_then(|i| { - self.char_index = i; - assert!(i < self.store.char_len()); - let entry = self.store.entry_buffer[i.to_usize()]; - if entry.is_simple() { - Some((self.char_index, GlyphInfo::Simple(self.store, i))) - } else { - // Fall back to the slow path. - self.next_complex_glyph(&entry, i) - } - }) + CharIndex(1) + }; + let i = self.char_index; + if !self.char_range.contains(i) { + return None + } + debug_assert!(i < self.store.char_len()); + let entry = self.store.entry_buffer[i.to_usize()]; + if entry.is_simple() { + Some((i, GlyphInfo::Simple(self.store, i))) + } else { + // Fall back to the slow path. + self.next_complex_glyph(&entry, i) } } } diff --git a/components/gfx/text/shaping/harfbuzz.rs b/components/gfx/text/shaping/harfbuzz.rs index 74a3b4e6d25..fbc8e450875 100644 --- a/components/gfx/text/shaping/harfbuzz.rs +++ b/components/gfx/text/shaping/harfbuzz.rs @@ -9,7 +9,7 @@ use font::{IGNORE_LIGATURES_SHAPING_FLAG, RTL_FLAG, ShapingOptions}; use platform::font::FontTable; use text::glyph::{CharIndex, GlyphStore, GlyphId, GlyphData}; use text::shaping::ShaperMethods; -use text::util::{float_to_fixed, fixed_to_float}; +use text::util::{float_to_fixed, fixed_to_float, is_bidi_control}; use euclid::Point2D; use harfbuzz::{HB_MEMORY_MODE_READONLY, HB_DIRECTION_LTR, HB_DIRECTION_RTL}; @@ -279,7 +279,11 @@ impl Shaper { // GlyphStore records are indexed by character, not byte offset. // so, we must be careful to increment this when saving glyph entries. - let mut char_idx = CharIndex(0); + let (mut char_idx, char_step) = if options.flags.contains(RTL_FLAG) { + (CharIndex(char_max as isize - 1), CharIndex(-1)) + } else { + (CharIndex(0), CharIndex(1)) + }; debug!("Shaped text[char count={}], got back {} glyph info records.", char_max, @@ -318,10 +322,10 @@ impl Shaper { debug!("{} -> {}", i, loc); } - debug!("text: {}", text); + debug!("text: {:?}", text); debug!("(char idx): char->(glyph index):"); for (i, ch) in text.char_indices() { - debug!("{}: {} --> {}", i, ch, byte_to_glyph[i]); + debug!("{}: {:?} --> {}", i, ch, byte_to_glyph[i]); } // some helpers @@ -453,16 +457,20 @@ impl Shaper { // // NB: When we acquire the ability to handle ligatures that cross word boundaries, // we'll need to do something special to handle `word-spacing` properly. - let shape = glyph_data.get_entry_for_glyph(glyph_span.begin(), &mut y_pos); let character = text.char_at(char_byte_span.begin()); - let advance = self.advance_for_shaped_glyph(shape.advance, character, options); - let data = GlyphData::new(shape.codepoint, - advance, - shape.offset, - false, - true, - true); - glyphs.add_glyph_for_char_index(char_idx, Some(character), &data); + if is_bidi_control(character) { + glyphs.add_nonglyph_for_char_index(char_idx, false, false); + } else { + let shape = glyph_data.get_entry_for_glyph(glyph_span.begin(), &mut y_pos); + let advance = self.advance_for_shaped_glyph(shape.advance, character, options); + let data = GlyphData::new(shape.codepoint, + advance, + shape.offset, + false, + true, + true); + glyphs.add_glyph_for_char_index(char_idx, Some(character), &data); + } } else { // collect all glyphs to be assigned to the first character. let mut datas = vec!(); @@ -488,7 +496,7 @@ impl Shaper { drop(range.ch); i = range.next; if i >= covered_byte_span.end() { break; } - char_idx = char_idx + CharIndex(1); + char_idx = char_idx + char_step; glyphs.add_nonglyph_for_char_index(char_idx, false, false); } } @@ -498,7 +506,7 @@ impl Shaper { glyph_span.reset(end, 0); let end = char_byte_span.end();; // FIXME: borrow checker workaround char_byte_span.reset(end, 0); - char_idx = char_idx + CharIndex(1); + char_idx = char_idx + char_step; } // this must be called after adding all glyph data; it sorts the diff --git a/components/gfx/text/util.rs b/components/gfx/text/util.rs index 59f3010c87d..1b78e42e792 100644 --- a/components/gfx/text/util.rs +++ b/components/gfx/text/util.rs @@ -92,9 +92,9 @@ pub fn transform_text(text: &str, } } - fn is_always_discardable_char(_ch: char) -> bool { - // TODO: check for bidi control chars, soft hyphens. - false + fn is_always_discardable_char(ch: char) -> bool { + // TODO: check for soft hyphens. + is_bidi_control(ch) } } @@ -105,3 +105,12 @@ pub fn float_to_fixed(before: usize, f: f64) -> i32 { pub fn fixed_to_float(before: usize, f: i32) -> f64 { f as f64 * 1.0f64 / ((1i32 << before) as f64) } + +pub fn is_bidi_control(c: char) -> bool { + match c { + '\u{202A}'...'\u{202E}' => true, + '\u{2066}'...'\u{2069}' => true, + '\u{200E}' | '\u{200F}' | '\u{061C}' => true, + _ => false + } +} diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 89e88847a22..519046b5533 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -194,6 +194,12 @@ struct InlineFragmentsAccumulator { /// Whether we've created a range to enclose all the fragments. This will be Some() if the /// outer node is an inline and None otherwise. enclosing_node: Option, + + /// Restyle damage to use for fragments created in this node. + restyle_damage: RestyleDamage, + + /// Bidi control characters to insert before and after these fragments. + bidi_control_chars: Option<(&'static str, &'static str)>, } impl InlineFragmentsAccumulator { @@ -201,6 +207,8 @@ impl InlineFragmentsAccumulator { InlineFragmentsAccumulator { fragments: IntermediateInlineFragments::new(), enclosing_node: None, + bidi_control_chars: None, + restyle_damage: RestyleDamage::empty(), } } @@ -211,6 +219,8 @@ impl InlineFragmentsAccumulator { address: node.opaque(), style: node.style().clone(), }), + bidi_control_chars: None, + restyle_damage: node.restyle_damage(), } } @@ -227,6 +237,8 @@ impl InlineFragmentsAccumulator { let InlineFragmentsAccumulator { mut fragments, enclosing_node, + bidi_control_chars, + restyle_damage, } = self; if let Some(enclosing_node) = enclosing_node { let frag_len = fragments.fragments.len(); @@ -239,6 +251,15 @@ impl InlineFragmentsAccumulator { frag.add_inline_context_style(enclosing_node.clone(), is_first, is_last); } + + // Control characters are later discarded in transform_text, so they don't affect the + // is_first/is_last styles above. + if let Some((start, end)) = bidi_control_chars { + fragments.fragments.push_front( + control_chars_to_fragment(&enclosing_node, start, restyle_damage)); + fragments.fragments.push_back( + control_chars_to_fragment(&enclosing_node, end, restyle_damage)); + } } fragments } @@ -744,6 +765,8 @@ impl<'a> FlowConstructor<'a> { -> ConstructionResult { let mut opt_inline_block_splits: LinkedList = LinkedList::new(); let mut fragment_accumulator = InlineFragmentsAccumulator::from_inline_node(node); + fragment_accumulator.bidi_control_chars = bidi_control_chars(&*node.style()); + let mut abs_descendants = Descendants::new(); // Concatenate all the fragments of our kids, creating {ib} splits as necessary. @@ -1631,3 +1654,37 @@ pub fn strip_ignorable_whitespace_from_end(this: &mut LinkedList) { drop(this.pop_back()); } } + +/// If the 'unicode-bidi' property has a value other than 'normal', return the bidi control codes +/// to inject before and after the text content of the element. +fn bidi_control_chars(style: &Arc) -> Option<(&'static str, &'static str)> { + use style::computed_values::unicode_bidi::T::*; + use style::computed_values::direction::T::*; + + let unicode_bidi = style.get_text().unicode_bidi; + let direction = style.get_inheritedbox().direction; + + // See the table in http://dev.w3.org/csswg/css-writing-modes/#unicode-bidi + match (unicode_bidi, direction) { + (normal, _) => None, + (embed, ltr) => Some(("\u{202A}", "\u{202C}")), + (embed, rtl) => Some(("\u{202B}", "\u{202C}")), + (isolate, ltr) => Some(("\u{2066}", "\u{2069}")), + (isolate, rtl) => Some(("\u{2067}", "\u{2069}")), + (bidi_override, ltr) => Some(("\u{202D}", "\u{202C}")), + (bidi_override, rtl) => Some(("\u{202E}", "\u{202C}")), + (isolate_override, ltr) => Some(("\u{2068}\u{202D}", "\u{202C}\u{2069}")), + (isolate_override, rtl) => Some(("\u{2068}\u{202E}", "\u{202C}\u{2069}")), + (plaintext, _) => Some(("\u{2068}", "\u{2069}")), + } +} + +fn control_chars_to_fragment(node: &InlineFragmentNodeInfo, text: &str, + restyle_damage: RestyleDamage) -> Fragment { + let info = SpecificFragmentInfo::UnscannedText( + UnscannedTextFragmentInfo::from_text(String::from(text))); + Fragment::from_opaque_node_and_style(node.address, + node.style.clone(), + restyle_damage, + info) +} diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 475535bbe97..ee8be133af0 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -1621,7 +1621,7 @@ impl Fragment { } match self.specific { SpecificFragmentInfo::UnscannedText(ref text_fragment_info) => { - util::str::is_whitespace(&text_fragment_info.text) + is_whitespace(&text_fragment_info.text) } _ => false, } diff --git a/components/script/dom/webidls/CSSStyleDeclaration.webidl b/components/script/dom/webidls/CSSStyleDeclaration.webidl index 16fc0c63a6a..85506b8173c 100644 --- a/components/script/dom/webidls/CSSStyleDeclaration.webidl +++ b/components/script/dom/webidls/CSSStyleDeclaration.webidl @@ -99,6 +99,7 @@ partial interface CSSStyleDeclaration { [TreatNullAs=EmptyString] attribute DOMString backfaceVisibility; [TreatNullAs=EmptyString] attribute DOMString direction; + [TreatNullAs=EmptyString] attribute DOMString unicodeBidi; [TreatNullAs=EmptyString] attribute DOMString filter; diff --git a/components/style/properties.mako.rs b/components/style/properties.mako.rs index 23618284757..4010f46307f 100644 --- a/components/style/properties.mako.rs +++ b/components/style/properties.mako.rs @@ -2076,6 +2076,8 @@ pub mod longhands { ${new_style_struct("Text", is_inherited=False)} + ${single_keyword("unicode-bidi", "normal embed isolate bidi-override isolate-override plaintext")} + <%self:longhand name="text-decoration" custom_cascade="True"> use cssparser::ToCss; use std::fmt; diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-001.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-001.htm.ini deleted file mode 100644 index 51fcc37e0c7..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-001.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-001.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-005b.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-005b.htm.ini deleted file mode 100644 index 121ce6fc5f6..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-005b.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-005b.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-006b.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-006b.htm.ini deleted file mode 100644 index 21eacdb847c..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-006b.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-006b.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-007b.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-007b.htm.ini deleted file mode 100644 index 9fb4efa2c70..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-007b.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-007b.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-008b.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-008b.htm.ini deleted file mode 100644 index 9078eee2a96..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-008b.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-008b.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-010b.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-010b.htm.ini deleted file mode 100644 index 55d48142534..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-010b.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-010b.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-box-model-006.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-box-model-006.htm.ini deleted file mode 100644 index 9ab9b4e6e3e..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-box-model-006.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-box-model-006.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/bidi-inline-001.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/bidi-inline-001.htm.ini deleted file mode 100644 index 185562ec797..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/bidi-inline-001.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bidi-inline-001.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/inline-formatting-context-007.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/inline-formatting-context-007.htm.ini index 8bc13abc629..b9f024e1851 100644 --- a/tests/wpt/metadata-css/css21_dev/html4/inline-formatting-context-007.htm.ini +++ b/tests/wpt/metadata-css/css21_dev/html4/inline-formatting-context-007.htm.ini @@ -1,3 +1,4 @@ [inline-formatting-context-007.htm] type: reftest - expected: FAIL + expected: + if os == "mac": FAIL diff --git a/tests/wpt/metadata-css/css21_dev/html4/unicode-bidi-002.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/unicode-bidi-002.htm.ini deleted file mode 100644 index 3f0ec3ccfef..00000000000 --- a/tests/wpt/metadata-css/css21_dev/html4/unicode-bidi-002.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[unicode-bidi-002.htm] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html.ini b/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html.ini index 00381dabe51..0d304a3f007 100644 --- a/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html.ini +++ b/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html.ini @@ -2,4 +2,5 @@ type: reftest reftype: == refurl: /html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested-ref.html - expected: FAIL + expected: + if os != "mac": FAIL diff --git a/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdo-element/bdo-override.html.ini b/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdo-element/bdo-override.html.ini deleted file mode 100644 index 1093b89f0c6..00000000000 --- a/tests/wpt/metadata/html/semantics/text-level-semantics/the-bdo-element/bdo-override.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[bdo-override.html] - type: reftest - expected: FAIL