Changeset 94775 in webkit


Ignore:
Timestamp:
Sep 8, 2011 11:46:01 AM (13 years ago)
Author:
eric@webkit.org
Message:

Reviewed by Ryosuke Niwa.

[BiDi] [CSS3] MASTER: Add support for the unicode-bidi:isolate CSS property
​https://bugs.webkit.org/show_bug.cgi?id=50912

Source/WebCore:

This patch adds support for CSS3 unicode-bidi: isolate property, under the -webkit- vendor prefix.
Parsing support was added in a previous patch, this wires up the RenderStyle values
to code changes in the BidiResolver.

The effect of this patch is that it makes it possible to "isolate" runs of text
so that their RTL-ness or LTR-ness does not bleed out into the rest of your text
and effect layout. This is important because many unicode characters (like parenthesis, ':', '-', etc.)
do not have intrinsic directionality and are affected by whatever characters come before/after.
If you have usernames which include RTL text, if you inject those usernames in your page
you might end up with nearby characters moving!
(like 'RTL USERNAME - my awesome site' as a title, could end up as
'my awesome site - USERNAME RTL' when correct would be 'USERNAME RTL - my awesome site'.)
This patch makes it possible to wrap sections of text in isolated spans, so that
they correctly order all their RTL/LTR contents, but also correctly participate in the
larger RTL/LTR ordering without affecting nearby characters.

Because much of this code is old and rarely touched, I've included extra background
information in hopes of expanding my set of potential reviewers:

WebKit uses the standard "Unicode Bidi Algorithm" henceforth known as the UBA.
The UBA is defined at ​http://unicode.org/reports/tr9/ for those not faint of heart.

Text layout is done per-block (<div>, <p>, etc), and begins with a string of text
(which in our case comes from the rendering tree) and a specified width.
First: Text is measured and wrapped into lines.
Second: The UBA is run over the lines of text.
Third: WebKit builds InlineBoxes (its linebox tree) and eventually render the text.

This patch modifies our UBA to ignore all text content inside "isolated" inlines (treating them as neutral characters)
and then adds another step after running the UBA, where we run the UBA recursively on any
previously identified "isolated" content.

The result of the UBA is an ordered list of "runs" of text with the RTL runs
correctly RTL and the LTR runs LTR.

The UBA does three things:

  1. It assigns a "class" to each character in a text stream (like neutral, strongly-RTL, strongly-LTR, etc.)
  2. Divides the text stream up into "runs" of characters of the same directionality (all RTL, all LTR).
  3. Re-orders those runs.

The UBA in WebKit is implemented by BidiResolver<T> in BidiResolver.h

The InlineBidiResolver (BidiResolver specialization which knows about the rendering tree)
walks along its InlineIterators, looking at each character and running the
Unicode Bidi Algorithm (UBA). It walks through the rendering tree subtree under
a block, using a (poorly named) bidiNext function which returns the next inline object.
Each inline object (or text character there-in) has a corresponding meaning in the UBA
such as a "strong RTL" character or a "neutral" character. The UBA reads these sequence
of characters, and figures out what direction (RTL or LTR) to assign to any neutral
characters it encounters, based on surrounding characters.

As the InlineBidiResolver is walking the rendering tree, the InlineIterator::advance()
function calls bidiNext(), which in turn can call notifyObserverEnteredObject/notifyObserverWillExitObject
notifying InlineBidiResolver that it is entering or exiting an "isolated"
span, at which point it will either start or stop ignoring the stream of characters
from the InlineIterator. When the InlineBidiResolver is ignoring the stream of
characters, instead of creating separate BidiRuns at each RTL/LTR boundary
as it normally would, it instead creates one "fake" run for the entire
isolated span. These fake runs participate in the normal UBA run ordering process,
but after the main UBA, a second pass is made where we examine
the list of isolatedRuns() and run the UBA on each of them, replacing the fake
run we previously inserted, with the resulting list of runs from that inner UBA run.
The way it "ignores" characters is by treating them all as neutral when inside an isolate.
Thus all the characters end up grouped in a single run, but their directionality (as a group)
is correctly affected by any surrounding strong characters.

If you understood that last paragraph, than the rest of the change is just plumbing.

I added a huge number of FIXMEs to this code, because this code has a variety of
design choices (or lack there of) which make some of this very difficult.

For example the bidiNext iterator function has two sets of mutually exclusive
parameters and can be used optionally with or without an observer. Prior to this
change there was only ever one object which cared about observing a walk over inlines
and that was InlineBidiResolver. This patch (regretfully) templatizes bidiNext
to support a new Observer type. The correct fix would be to rip bidiNext into
multiple functions and rip need for observation out of InlineBidiResolver.
Unfortunately I've tried both in separate bugs and failed. This code is very very
old and very poorly understood. We're slowly moving forward, this is another tiny step.

This is my fourth iteration of this patch (I'm happy to do more!), but I believe
it's a good compromise between fixing all of the design gotcha's of our bidi
system and doing the minimum amount to add this killer CSS feature.

I ran the PLT. (It averaged 0.2% faster with this change, but I attribute that to noise).

Test: css3/unicode-bidi-isolate-basic.html and css3/unicode-bidi-isolate-aharon.html

  • platform/text/BidiResolver.h:

(WebCore::BidiCharacterRun::setNext):

  • Needed by the new replaceRunWithRuns function.

(WebCore::BidiResolver::BidiResolver):
(WebCore::BidiResolver::~BidiResolver):
(WebCore::BidiResolver::enterIsolate):
(WebCore::BidiResolver::exitIsolate):
(WebCore::BidiResolver::inIsolate):
(WebCore::BidiResolver::isolatedRuns):

  • Used to track isolated spans of text as they're encoutered. They're stuffed away here to be processed recursively after the main UBA has done its thang.

(WebCore::::appendRun):
(WebCore::::embed):
(WebCore::::commitExplicitEmbedding):
(WebCore::::createBidiRunsForLine):

  • platform/text/BidiRunList.h:

(WebCore::::replaceRunWithRuns):

  • This effectively takes all the runs from one runlist and adds them to this one, replacing the fake run we inserted during a previous pass of the UBA.
  • This RunList now owns the runs, so we call clear() on the other RunList so that we don't end up double-freeing the runs.

(WebCore::::clear):

  • This allows us to "take" runs from another run list and then clear it.
  • rendering/BidiRun.h:

(WebCore::BidiRun::object):

  • rendering/InlineIterator.h:

(WebCore::InlineIterator::object):
(WebCore::InlineIterator::offset):
(WebCore::notifyObserverEnteredObject): Mostly just renaming and adding a FIXME about plaintext.
(WebCore::notifyObserverWillExitObject): Mostly just renaming.
(WebCore::addPlaceholderRunForIsolatedInline):
(WebCore::isIsolatedInline):
(WebCore::InlineBidiResolver::appendRun):

  • rendering/RenderBlockLineLayout.cpp:

(WebCore::statusWithDirection):
(WebCore::constructBidiRuns):

  • This is the heavy-lifting of this change. This function runs the UBA recursively on all the previously identified isolated spans.
  • If we encounter more isolated spans in our run, we just add them to the main list an keep going. Because the runs are linked lists and we have direct pointers to our placeholder objects, we don't care what order we process the placeholders in, so long as when we're done, they're all processed.

(WebCore::RenderBlock::layoutInlineChildren):

LayoutTests:

Two new tests for testing unicode-bidi: isolate behavior.
Note that the test from Aharon Lanin has one failing subtest
I've asked him if the test might have a typo in:
​https://bugs.webkit.org/show_bug.cgi?id=50912#c30

  • css3/unicode-bidi-isolate-aharon.html: Added.
    • Some various unicode-bidi: isolate tests from Aharon.
  • css3/unicode-bidi-isolate-basic.html: Added.
    • This test tries all possible orderings of strong-LTR, strong-RTL and neutral characters across unicode-bidi: isolate spans to make sure that we match expected rendering.
    • A little red bleeds through the test, but that appears to be from anti-aliasing and possible automatic font kerning, not layout failures.
  • platform/mac/css3/unicode-bidi-isolate-aharon-expected.png: Added.
  • platform/mac/css3/unicode-bidi-isolate-aharon-expected.txt: Added.
  • platform/mac/css3/unicode-bidi-isolate-basic-expected.png: Added.
  • platform/mac/css3/unicode-bidi-isolate-basic-expected.txt: Added.
Location:
trunk
Files:
6 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r94773 r94775  
     12011-04-19  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        [BiDi] [CSS3] MASTER: Add support for the unicode-bidi:isolate CSS property
     6        https://bugs.webkit.org/show_bug.cgi?id=50912
     7
     8        Two new tests for testing unicode-bidi: isolate behavior.
     9        Note that the test from Aharon Lanin has one failing subtest
     10        I've asked him if the test might have a typo in:
     11        https://bugs.webkit.org/show_bug.cgi?id=50912#c30
     12
     13        * css3/unicode-bidi-isolate-aharon.html: Added.
     14         - Some various unicode-bidi: isolate tests from Aharon.
     15        * css3/unicode-bidi-isolate-basic.html: Added.
     16         - This test tries all possible orderings of strong-LTR, strong-RTL and neutral characters
     17           across unicode-bidi: isolate spans to make sure that we match expected rendering.
     18         - A little red bleeds through the test, but that appears to be from anti-aliasing
     19           and possible automatic font kerning, not layout failures.
     20        * platform/mac/css3/unicode-bidi-isolate-aharon-expected.png: Added.
     21        * platform/mac/css3/unicode-bidi-isolate-aharon-expected.txt: Added.
     22        * platform/mac/css3/unicode-bidi-isolate-basic-expected.png: Added.
     23        * platform/mac/css3/unicode-bidi-isolate-basic-expected.txt: Added.
     24
    1252011-09-08  Luke Macpherson   <macpherson@chromium.org>
    226

  • trunk/Source/WebCore/ChangeLog

    r94771 r94775  
     12011-04-19  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        [BiDi] [CSS3] MASTER: Add support for the unicode-bidi:isolate CSS property
     6        https://bugs.webkit.org/show_bug.cgi?id=50912
     7
     8        This patch adds support for CSS3 unicode-bidi: isolate property, under the -webkit- vendor prefix.
     9        Parsing support was added in a previous patch, this wires up the RenderStyle values
     10        to code changes in the BidiResolver.
     11
     12        The effect of this patch is that it makes it possible to "isolate" runs of text
     13        so that their RTL-ness or LTR-ness does not bleed out into the rest of your text
     14        and effect layout.  This is important because many unicode characters (like parenthesis, ':', '-', etc.)
     15        do not have intrinsic directionality and are affected by whatever characters come before/after.
     16        If you have usernames which include RTL text, if you inject those usernames in your page
     17        you might end up with nearby characters moving!
     18        (like 'RTL USERNAME - my awesome site' as a title, could end up as
     19        'my awesome site - USERNAME RTL' when correct would be 'USERNAME RTL - my awesome site'.)
     20        This patch makes it possible to wrap sections of text in isolated spans, so that
     21        they correctly order all their RTL/LTR contents, but also correctly participate in the
     22        larger RTL/LTR ordering without affecting nearby characters.
     23
     24        Because much of this code is old and rarely touched, I've included extra background
     25        information in hopes of expanding my set of potential reviewers:
     26
     27        WebKit uses the standard "Unicode Bidi Algorithm" henceforth known as the UBA.
     28        The UBA is defined at http://unicode.org/reports/tr9/ for those not faint of heart.
     29
     30        Text layout is done per-block (<div>, <p>, etc), and begins with a string of text
     31        (which in our case comes from the rendering tree) and a specified width.
     32        First:  Text is measured and wrapped into lines.
     33        Second: The UBA is run over the lines of text.
     34        Third:  WebKit builds InlineBoxes (its linebox tree) and eventually render the text.
     35
     36        This patch modifies our UBA to ignore all text content inside "isolated" inlines (treating them as neutral characters)
     37        and then adds another step after running the UBA, where we run the UBA recursively on any
     38        previously identified "isolated" content.
     39
     40        The result of the UBA is an ordered list of "runs" of text with the RTL runs
     41        correctly RTL and the LTR runs LTR.
     42
     43        The UBA does three things:
     44        1.  It assigns a "class" to each character in a text stream (like neutral, strongly-RTL, strongly-LTR, etc.)
     45        2.  Divides the text stream up into "runs" of characters of the same directionality (all RTL, all LTR).
     46        3.  Re-orders those runs.
     47
     48        The UBA in WebKit is implemented by BidiResolver<T> in BidiResolver.h
     49
     50        The InlineBidiResolver (BidiResolver specialization which knows about the rendering tree)
     51        walks along its InlineIterators, looking at each character and running the
     52        Unicode Bidi Algorithm (UBA).  It walks through the rendering tree subtree under
     53        a block, using a (poorly named) bidiNext function which returns the next inline object.
     54        Each inline object (or text character there-in) has a corresponding meaning in the UBA
     55        such as a "strong RTL" character or a "neutral" character.  The UBA reads these sequence
     56        of characters, and figures out what direction (RTL or LTR) to assign to any neutral
     57        characters it encounters, based on surrounding characters.
     58
     59        As the InlineBidiResolver is walking the rendering tree, the InlineIterator::advance()
     60        function calls bidiNext(), which in turn can call notifyObserverEnteredObject/notifyObserverWillExitObject
     61        notifying InlineBidiResolver that it is entering or exiting an "isolated"
     62        span, at which point it will either start or stop ignoring the stream of characters
     63        from the InlineIterator.  When the InlineBidiResolver is ignoring the stream of
     64        characters, instead of creating separate BidiRuns at each RTL/LTR boundary
     65        as it normally would, it instead creates one "fake" run for the entire
     66        isolated span.  These fake runs participate in the normal UBA run ordering process,
     67        but after the main UBA, a second pass is made where we examine
     68        the list of isolatedRuns() and run the UBA on each of them, replacing the fake
     69        run we previously inserted, with the resulting list of runs from that inner UBA run.
     70        The way it "ignores" characters is by treating them all as neutral when inside an isolate.
     71        Thus all the characters end up grouped in a single run, but their directionality (as a group)
     72        is correctly affected by any surrounding strong characters.
     73
     74        If you understood that last paragraph, than the rest of the change is just plumbing.
     75
     76        I added a huge number of FIXMEs to this code, because this code has a variety of
     77        design choices (or lack there of) which make some of this very difficult.
     78
     79        For example the bidiNext iterator function has two sets of mutually exclusive
     80        parameters and can be used optionally with or without an observer.  Prior to this
     81        change there was only ever one object which cared about observing a walk over inlines
     82        and that was InlineBidiResolver.  This patch (regretfully) templatizes bidiNext
     83        to support a new Observer type.  The correct fix would be to rip bidiNext into
     84        multiple functions and rip need for observation out of InlineBidiResolver.
     85        Unfortunately I've tried both in separate bugs and failed.  This code is very very
     86        old and very poorly understood.  We're slowly moving forward, this is another tiny step.
     87
     88        This is my fourth iteration of this patch (I'm happy to do more!), but I believe
     89        it's a good compromise between fixing all of the design gotcha's of our bidi
     90        system and doing the minimum amount to add this killer CSS feature.
     91
     92        I ran the PLT.  (It averaged 0.2% faster with this change, but I attribute that to noise).
     93
     94        Test: css3/unicode-bidi-isolate-basic.html and css3/unicode-bidi-isolate-aharon.html
     95
     96        * platform/text/BidiResolver.h:
     97        (WebCore::BidiCharacterRun::setNext):
     98         - Needed by the new replaceRunWithRuns function.
     99        (WebCore::BidiResolver::BidiResolver):
     100        (WebCore::BidiResolver::~BidiResolver):
     101        (WebCore::BidiResolver::enterIsolate):
     102        (WebCore::BidiResolver::exitIsolate):
     103        (WebCore::BidiResolver::inIsolate):
     104        (WebCore::BidiResolver::isolatedRuns):
     105         - Used to track isolated spans of text as they're encoutered.
     106           They're stuffed away here to be processed recursively
     107           after the main UBA has done its thang.
     108        (WebCore::::appendRun):
     109        (WebCore::::embed):
     110        (WebCore::::commitExplicitEmbedding):
     111        (WebCore::::createBidiRunsForLine):
     112        * platform/text/BidiRunList.h:
     113        (WebCore::::replaceRunWithRuns):
     114         - This effectively takes all the runs from one runlist and adds them to
     115           this one, replacing the fake run we inserted during a previous pass of the UBA.
     116         - This RunList now owns the runs, so we call clear() on the other RunList
     117           so that we don't end up double-freeing the runs.
     118        (WebCore::::clear):
     119         - This allows us to "take" runs from another run list and then clear it.
     120        * rendering/BidiRun.h:
     121        (WebCore::BidiRun::object):
     122        * rendering/InlineIterator.h:
     123        (WebCore::InlineIterator::object):
     124        (WebCore::InlineIterator::offset):
     125        (WebCore::notifyObserverEnteredObject): Mostly just renaming and adding a FIXME about plaintext.
     126        (WebCore::notifyObserverWillExitObject): Mostly just renaming.
     127        (WebCore::addPlaceholderRunForIsolatedInline):
     128        (WebCore::isIsolatedInline):
     129        (WebCore::InlineBidiResolver::appendRun):
     130        * rendering/RenderBlockLineLayout.cpp:
     131        (WebCore::statusWithDirection):
     132        (WebCore::constructBidiRuns):
     133         - This is the heavy-lifting of this change.  This function
     134           runs the UBA recursively on all the previously identified isolated spans.
     135         - If we encounter more isolated spans in our run, we just add them to the
     136           main list an keep going.  Because the runs are linked lists and we have
     137           direct pointers to our placeholder objects, we don't care what order
     138           we process the placeholders in, so long as when we're done, they're all processed.
     139        (WebCore::RenderBlock::layoutInlineChildren):
     140
    11412011-09-08  Kentaro Hara  <haraken@google.com>
    2142

  • trunk/Source/WebCore/platform/text/BidiResolver.h

    r85143 r94775  
    145145
    146146    BidiCharacterRun* next() const { return m_next; }
     147    void setNext(BidiCharacterRun* next) { m_next = next; }
    147148
    148149    unsigned char m_level;
    … …  
    168169        , m_reachedEndOfLine(false)
    169170        , m_emptyRun(true)
     171        , m_nestedIsolateCount(0)
    170172    {
    171173    }
     174
     175#ifndef NDEBUG
     176    ~BidiResolver();
     177#endif
    172178
    173179    const Iterator& position() const { return m_current; }
    … …  
    191197    MidpointState<Iterator>& midpointState() { return m_midpointState; }
    192198
     199    // The current algorithm handles nested isolates one layer of nesting at a time.
     200    // But when we layout each isolated span, we will walk into (and ignore) all
     201    // child isolated spans.
     202    void enterIsolate() { m_nestedIsolateCount++; }
     203    void exitIsolate() { ASSERT(m_nestedIsolateCount >= 1); m_nestedIsolateCount--; }
     204    bool inIsolate() const { return m_nestedIsolateCount; }
     205
    193206    void embed(WTF::Unicode::Direction, BidiEmbeddingSource);
    194207    bool commitExplicitEmbedding();
    … …  
    201214    // It's unclear if this is still needed.
    202215    void markCurrentRunEmpty() { m_emptyRun = true; }
     216
     217    Vector<Run*>& isolatedRuns() { return m_isolatedRuns; }
    203218
    204219protected:
    … …  
    210225    // sor and eor are "start of run" and "end of run" respectively and correpond
    211226    // to abreviations used in UBA spec: http://unicode.org/reports/tr9/#BD7
    212     Iterator m_sor;
    213     Iterator m_eor;
     227    Iterator m_sor; // Points to the first character in the current run.
     228    Iterator m_eor; // Points to the last character in the current run.
    214229    Iterator m_last;
    215230    BidiStatus m_status;
    … …  
    226241    MidpointState<Iterator> m_midpointState;
    227242
     243    unsigned m_nestedIsolateCount;
     244    Vector<Run*> m_isolatedRuns;
     245
    228246private:
    229247    void raiseExplicitEmbeddingLevel(WTF::Unicode::Direction from, WTF::Unicode::Direction to);
    … …  
    237255};
    238256
     257#ifndef NDEBUG
     258template <class Iterator, class Run>
     259BidiResolver<Iterator, Run>::~BidiResolver()
     260{
     261    // The owner of this resolver should have handled the isolated runs
     262    // or should never have called enterIsolate().
     263    ASSERT(m_isolatedRuns.isEmpty());
     264    ASSERT(!m_nestedIsolateCount);
     265}
     266#endif
     267
    239268template <class Iterator, class Run>
    240269void BidiResolver<Iterator, Run>::appendRun()
    … …  
    263292void BidiResolver<Iterator, Run>::embed(WTF::Unicode::Direction dir, BidiEmbeddingSource source)
    264293{
     294    // Isolated spans compute base directionality during their own UBA run.
     295    // Do not insert fake embed characters once we enter an isolated span.
     296    ASSERT(!inIsolate());
    265297    using namespace WTF::Unicode;
    266298
    … …  
    366398bool BidiResolver<Iterator, Run>::commitExplicitEmbedding()
    367399{
     400    // This gets called from bidiFirst when setting up our start position.
     401    ASSERT(!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty());
     402
    368403    using namespace WTF::Unicode;
    369404
    … …  
    545580                dirCurrent = m_status.last;
    546581        }
     582
     583        // We ignore all character directionality while in unicode-bidi: isolate spans.
     584        // We'll handle ordering the isolated characters in a second pass.
     585        if (inIsolate())
     586            dirCurrent = OtherNeutral;
    547587
    548588        ASSERT(m_status.eor != OtherNeutral || m_eor.atEnd());

  • trunk/Source/WebCore/platform/text/BidiRunList.h

    r83240 r94775  
    6060    void setLogicallyLastRun(Run* run) { m_logicallyLastRun = run; }
    6161
     62    void replaceRunWithRuns(Run* toReplace, BidiRunList<Run>& newRuns);
     63
    6264private:
     65    void clearWithoutDestroyingRuns();
     66
    6367    Run* m_firstRun;
    6468    Run* m_lastRun;
    … …  
    135139    run->m_next = m_firstRun;
    136140    m_firstRun = run;
     141}
     142
     143template <class Run>
     144void BidiRunList<Run>::replaceRunWithRuns(Run* toReplace, BidiRunList<Run>& newRuns)
     145{
     146    ASSERT(newRuns.runCount());
     147    ASSERT(m_firstRun);
     148    ASSERT(toReplace);
     149
     150    if (m_firstRun == toReplace)
     151        m_firstRun = newRuns.firstRun();
     152    else {
     153        // Find the run just before "toReplace" in the list of runs.
     154        Run* previousRun = m_firstRun;
     155        while (previousRun->next() != toReplace)
     156            previousRun = previousRun->next();
     157        ASSERT(previousRun);
     158        previousRun->setNext(newRuns.firstRun());
     159    }
     160
     161    newRuns.lastRun()->setNext(toReplace->next());
     162
     163    // Fix up any of other pointers which may now be stale.
     164    if (m_lastRun == toReplace)
     165        m_lastRun = newRuns.lastRun();
     166    if (m_logicallyLastRun == toReplace)
     167        m_logicallyLastRun = newRuns.logicallyLastRun();
     168    m_runCount += newRuns.runCount() - 1; // We added the new runs and removed toReplace.
     169
     170    toReplace->destroy();
     171    newRuns.clearWithoutDestroyingRuns();
     172}
     173
     174template <class Run>
     175void BidiRunList<Run>::clearWithoutDestroyingRuns()
     176{
     177    m_firstRun = 0;
     178    m_lastRun = 0;
     179    m_logicallyLastRun = 0;
     180    m_runCount = 0;
    137181}
    138182

  • trunk/Source/WebCore/rendering/BidiRun.h

    r61548 r94775  
    5252
    5353    BidiRun* next() { return static_cast<BidiRun*>(m_next); }
     54    RenderObject* object() { return m_object; }
    5455
    5556private:

  • trunk/Source/WebCore/rendering/InlineIterator.h

    r93559 r94775  
    3232namespace WebCore {
    3333
     34// This class is used to RenderInline subtrees, stepping by character within the
     35// text children. InlineIterator will use bidiNext to find the next RenderText
     36// optionally notifying a BidiResolver every time it steps into/out of a RenderInline.
    3437class InlineIterator {
    3538public:
    … …  
    6467    }
    6568
     69    RenderObject* object() const { return m_obj; }
     70    unsigned offset() const { return m_pos; }
    6671    RenderObject* root() const { return m_root; }
    6772
    … …  
    113118}
    114119
    115 static inline void notifyResolverEnteredObject(InlineBidiResolver* resolver, RenderObject* object)
    116 {
    117     if (!resolver || !object || !object->isRenderInline())
     120template <class Observer>
     121static inline void notifyObserverEnteredObject(Observer* observer, RenderObject* object)
     122{
     123    if (!observer || !object || !object->isRenderInline())
    118124        return;
    119125
    120126    RenderStyle* style = object->style();
    121127    EUnicodeBidi unicodeBidi = style->unicodeBidi();
     128    if (unicodeBidi == UBNormal) {
     129        // http://dev.w3.org/csswg/css3-writing-modes/#unicode-bidi
     130        // "The element does not open an additional level of embedding with respect to the bidirectional algorithm."
     131        // Thus we ignore any possible dir= attribute on the span.
     132        return;
     133    }
     134    if (unicodeBidi == Isolate) {
     135        observer->enterIsolate();
     136        // Embedding/Override characters implied by dir= are handled when
     137        // we process the isolated span, not when laying out the "parent" run.
     138        return;
     139    }
     140
     141    // FIXME: Should unicode-bidi: plaintext really be embedding override/embed characters here?
     142    observer->embed(embedCharFromDirection(style->direction(), unicodeBidi), FromStyleOrDOM);
     143}
     144
     145template <class Observer>
     146static inline void notifyObserverWillExitObject(Observer* observer, RenderObject* object)
     147{
     148    if (!observer || !object || !object->isRenderInline())
     149        return;
     150
     151    EUnicodeBidi unicodeBidi = object->style()->unicodeBidi();
    122152    if (unicodeBidi == UBNormal)
     153        return; // Nothing to do for unicode-bidi: normal
     154    if (unicodeBidi == Isolate) {
     155        observer->exitIsolate();
    123156        return;
    124     resolver->embed(embedCharFromDirection(style->direction(), unicodeBidi), FromStyleOrDOM);
    125 }
    126 
    127 static inline void notifyResolverWillExitObject(InlineBidiResolver* resolver, RenderObject* object)
    128 {
    129     if (!resolver || !object || !object->isRenderInline())
    130         return;
    131     if (object->style()->unicodeBidi() == UBNormal)
    132         return;
    133     resolver->embed(WTF::Unicode::PopDirectionalFormat, FromStyleOrDOM);
     157    }
     158
     159    // Otherwise we pop any embed/override character we added when we opened this tag.
     160    observer->embed(WTF::Unicode::PopDirectionalFormat, FromStyleOrDOM);
    134161}
    135162
    … …  
    149176// This function will iterate over inlines within a block, optionally notifying
    150177// a bidi resolver as it enters/exits inlines (so it can push/pop embedding levels).
    151 static inline RenderObject* bidiNextShared(RenderObject* root, RenderObject* current, InlineBidiResolver* resolver = 0, EmptyInlineBehavior emptyInlineBehavior = SkipEmptyInlines, bool* endOfInlinePtr = 0)
     178template <class Observer>
     179static inline RenderObject* bidiNextShared(RenderObject* root, RenderObject* current, Observer* observer = 0, EmptyInlineBehavior emptyInlineBehavior = SkipEmptyInlines, bool* endOfInlinePtr = 0)
    152180{
    153181    RenderObject* next = 0;
    … …  
    160188        if (!oldEndOfInline && !isIteratorTarget(current)) {
    161189            next = current->firstChild();
    162             notifyResolverEnteredObject(resolver, next);
     190            notifyObserverEnteredObject(observer, next);
    163191        }
    164192
    … …  
    173201
    174202            while (current && current != root) {
    175                 notifyResolverWillExitObject(resolver, current);
     203                notifyObserverWillExitObject(observer, current);
    176204
    177205                next = current->nextSibling();
    178206                if (next) {
    179                     notifyResolverEnteredObject(resolver, next);
     207                    notifyObserverEnteredObject(observer, next);
    180208                    break;
    181209                }
    … …  
    206234}
    207235
    208 static inline RenderObject* bidiNextSkippingEmptyInlines(RenderObject* root, RenderObject* current, InlineBidiResolver* observer = 0)
     236template <class Observer>
     237static inline RenderObject* bidiNextSkippingEmptyInlines(RenderObject* root, RenderObject* current, Observer* observer)
    209238{
    210239    // The SkipEmptyInlines callers never care about endOfInlinePtr.
    211240    return bidiNextShared(root, current, observer, SkipEmptyInlines);
     241}
     242
     243// This makes callers cleaner as they don't have to specify a type for the observer when not providing one.
     244static inline RenderObject* bidiNextSkippingEmptyInlines(RenderObject* root, RenderObject* current)
     245{
     246    InlineBidiResolver* observer = 0;
     247    return bidiNextSkippingEmptyInlines(root, current, observer);
    212248}
    213249
    … …  
    226262
    227263    if (o->isRenderInline()) {
    228         notifyResolverEnteredObject(resolver, o);
     264        notifyObserverEnteredObject(resolver, o);
    229265        if (o->firstChild())
    230266            o = bidiNextSkippingEmptyInlines(root, o, resolver);
    … …  
    352388}
    353389
     390static inline bool isIsolatedInline(RenderObject* object)
     391{
     392    ASSERT(object);
     393    return object->isRenderInline() && object->style()->unicodeBidi() == Isolate;
     394}
     395
     396static inline RenderObject* containingIsolate(RenderObject* object, RenderObject* root)
     397{
     398    ASSERT(object);
     399    while (object && object != root) {
     400        if (isIsolatedInline(object))
     401            return object;
     402        object = object->parent();
     403    }
     404    return 0;
     405}
     406
     407// FIXME: This belongs on InlineBidiResolver, except it's a template specialization
     408// of BidiResolver which knows nothing about RenderObjects.
     409static inline void addPlaceholderRunForIsolatedInline(InlineBidiResolver& resolver, RenderObject* isolatedInline)
     410{
     411    ASSERT(isolatedInline);
     412    BidiRun* isolatedRun = new (isolatedInline->renderArena()) BidiRun(0, 0, isolatedInline, resolver.context(), resolver.dir());
     413    resolver.runs().addRun(isolatedRun);
     414    // FIXME: isolatedRuns() could be a hash of object->run and then we could cheaply
     415    // ASSERT here that we didn't create multiple objects for the same inline.
     416    resolver.isolatedRuns().append(isolatedRun);
     417}
     418
     419class IsolateTracker {
     420public:
     421    explicit IsolateTracker(bool inIsolate)
     422        : m_nestedIsolateCount(inIsolate ? 1 : 0)
     423        , m_haveAddedFakeRunForRootIsolate(false)
     424    {
     425    }
     426
     427    void enterIsolate() { m_nestedIsolateCount++; }
     428    void exitIsolate()
     429    {
     430        ASSERT(m_nestedIsolateCount >= 1);
     431        m_nestedIsolateCount--;
     432        if (!inIsolate())
     433            m_haveAddedFakeRunForRootIsolate = false;
     434    }
     435    bool inIsolate() const { return m_nestedIsolateCount; }
     436
     437    // We don't care if we encounter bidi directional overrides.
     438    void embed(WTF::Unicode::Direction, BidiEmbeddingSource) { }
     439
     440    void addFakeRunIfNecessary(RenderObject* obj, InlineBidiResolver& resolver)
     441    {
     442        // We only need to lookup the root isolated span and add a fake run
     443        // for it once, but we'll be called for every span inside the isolated span
     444        // so we just ignore the call.
     445        if (m_haveAddedFakeRunForRootIsolate)
     446            return;
     447        m_haveAddedFakeRunForRootIsolate = true;
     448
     449        // FIXME: position() could be outside the run and may be the wrong call here.
     450        // If we were passed an InlineIterator instead that would have the right root().
     451        RenderObject* isolatedInline = containingIsolate(obj, resolver.position().root());
     452        // FIXME: Because enterIsolate is not passed a RenderObject, we have to crawl up the
     453        // tree to see which parent inline is the isolate. We could change enterIsolate
     454        // to take a RenderObject and do this logic there, but that would be a layering
     455        // violation for BidiResolver (which knows nothing about RenderObject).
     456        addPlaceholderRunForIsolatedInline(resolver, isolatedInline);
     457    }
     458
     459private:
     460    unsigned m_nestedIsolateCount;
     461    bool m_haveAddedFakeRunForRootIsolate;
     462};
     463
    354464template <>
    355465inline void InlineBidiResolver::appendRun()
    356466{
    357467    if (!m_emptyRun && !m_eor.atEnd()) {
     468        // Keep track of when we enter/leave "unicode-bidi: isolate" inlines.
     469        // Initialize our state depending on if we're starting in the middle of such an inline.
     470        // FIXME: Could this initialize from this->inIsolate() instead of walking up the render tree?
     471        IsolateTracker isolateTracker(containingIsolate(m_sor.m_obj, m_sor.root()));
    358472        int start = m_sor.m_pos;
    359473        RenderObject* obj = m_sor.m_obj;
    360474        while (obj && obj != m_eor.m_obj && obj != endOfLine.m_obj) {
    361             RenderBlock::appendRunsForObject(m_runs, start, obj->length(), obj, *this);
     475            if (isolateTracker.inIsolate())
     476                isolateTracker.addFakeRunIfNecessary(obj, *this);
     477            else
     478                RenderBlock::appendRunsForObject(m_runs, start, obj->length(), obj, *this);
     479            // FIXME: start/obj should be an InlineIterator instead of two separate variables.
    362480            start = 0;
    363             obj = bidiNextSkippingEmptyInlines(m_sor.root(), obj);
     481            obj = bidiNextSkippingEmptyInlines(m_sor.root(), obj, &isolateTracker);
    364482        }
    365483        if (obj) {
    … …  
    371489            // It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be
    372490            int end = obj->length() ? pos + 1 : 0;
    373             RenderBlock::appendRunsForObject(m_runs, start, end, obj, *this);
     491            if (isolateTracker.inIsolate())
     492                isolateTracker.addFakeRunIfNecessary(obj, *this);
     493            else
     494                RenderBlock::appendRunsForObject(m_runs, start, end, obj, *this);
    374495        }
    375496

  • trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

    r94710 r94775  
    786786    floatingObject->m_originatingLine = lastRootBox();
    787787    lastRootBox()->appendFloat(floatingObject->renderer());
     788}
     789
     790// FIXME: This should be a BidiStatus constructor or create method.
     791static inline BidiStatus statusWithDirection(TextDirection textDirection)
     792{
     793    WTF::Unicode::Direction direction = textDirection == LTR ? LeftToRight : RightToLeft;
     794    RefPtr<BidiContext> context = BidiContext::create(textDirection == LTR ? 0 : 1, direction, false, FromStyleOrDOM);
     795
     796    // This copies BidiStatus and may churn the ref on BidiContext. I doubt it matters.
     797    return BidiStatus(direction, direction, direction, context.release());
     798}
     799
     800// FIXME: BidiResolver should have this logic.
     801static inline void constructBidiRuns(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfLine, VisualDirectionOverride override, bool previousLineBrokeCleanly)
     802{
     803    // FIXME: We should pass a BidiRunList into createBidiRunsForLine instead
     804    // of the resolver owning the runs.
     805    ASSERT(&topResolver.runs() == &bidiRuns);
     806    topResolver.createBidiRunsForLine(endOfLine, override, previousLineBrokeCleanly);
     807
     808    while (!topResolver.isolatedRuns().isEmpty()) {
     809        // It does not matter which order we resolve the runs as long as we resolve them all.
     810        BidiRun* isolatedRun = topResolver.isolatedRuns().last();
     811        topResolver.isolatedRuns().removeLast();
     812
     813        // Only inlines make sense with unicode-bidi: isolate (blocks are already isolated).
     814        RenderInline* isolatedSpan = toRenderInline(isolatedRun->object());
     815        InlineBidiResolver isolatedResolver;
     816        isolatedResolver.setStatus(statusWithDirection(isolatedSpan->style()->direction()));
     817
     818        // FIXME: The fact that we have to construct an Iterator here
     819        // currently prevents this code from moving into BidiResolver.
     820        RenderObject* startObj = bidiFirstSkippingEmptyInlines(isolatedSpan, &isolatedResolver);
     821        isolatedResolver.setPosition(InlineIterator(isolatedSpan, startObj, 0));
     822
     823        // FIXME: isolatedEnd should probably equal end or the last char in isolatedSpan.
     824        InlineIterator isolatedEnd = endOfLine;
     825        // FIXME: What should end and previousLineBrokeCleanly be?
     826        // rniwa says previousLineBrokeCleanly is just a WinIE hack and could always be false here?
     827        isolatedResolver.createBidiRunsForLine(isolatedEnd, NoVisualOverride, previousLineBrokeCleanly);
     828        // Note that we do not delete the runs from the resolver.
     829        bidiRuns.replaceRunWithRuns(isolatedRun, isolatedResolver.runs());
     830
     831        // If we encountered any nested isolate runs, just move them
     832        // to the top resolver's list for later processing.
     833        if (!isolatedResolver.isolatedRuns().isEmpty()) {
     834            topResolver.isolatedRuns().append(isolatedResolver.isolatedRuns());
     835            isolatedResolver.isolatedRuns().clear();
     836        }
     837    }
    788838}
    789839
    … …  
    10471097            // FIXME: This ownership is reversed. We should own the BidiRunList and pass it to createBidiRunsForLine.
    10481098            BidiRunList<BidiRun>& bidiRuns = resolver.runs();
    1049             resolver.createBidiRunsForLine(end, override, layoutState.lineInfo().previousLineBrokeCleanly());
     1099            constructBidiRuns(resolver, bidiRuns, end, override, layoutState.lineInfo().previousLineBrokeCleanly());
    10501100            ASSERT(resolver.position() == end);
    10511101
Note: See TracChangeset for help on using the changeset viewer.