Changeset 82424 in webkit


Ignore:
Timestamp:
Mar 30, 2011 5:48:08 AM (13 years ago)
Author:
leviw@chromium.org
Message:

2011-03-29 Levi Weintraub <leviw@chromium.org>

Reviewed by Eric Seidel.

RTL: Directionality always reset on hard line break
https://bugs.webkit.org/show_bug.cgi?id=23124

Testing that hard line breaks are being treated as paragraph separators but only
clearing directional state from Unicode control characters, not DOM nodes.
Also adding expected pixel results for bidi-override-in-anonymous-block as we're
passing more of the test with this patch.

  • fast/text/international/bidi-br-as-paragraph-separator.html: Added.
  • platform/mac/fast/css/bidi-override-in-anonymous-block-expected.checksum: Added.
  • platform/mac/fast/css/bidi-override-in-anonymous-block-expected.png: Added.
  • platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.checksum: Added.
  • platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.png: Added.
  • platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.txt: Added.

2011-03-30 Levi Weintraub <leviw@chromium.org>

Reviewed by Eric Seidel.

RTL: Directionality always reset on hard line break
https://bugs.webkit.org/show_bug.cgi?id=23124

No longer clearing all BidiContexts when we hit a hard line break.
Instead, directionality applied by DOM elements is preserved by
reconstructing the context stack ignoring those that didn't come
from the DOM.

Test: fast/text/international/bidi-br-as-paragraph-separator.html

  • platform/text/BidiContext.cpp: (WebCore::BidiContext::createUncached): (WebCore::BidiContext::create): (WebCore::copyContextAndRebaselineLevel): Helper to make a copy of a context and recalculate its bidi level. (WebCore::BidiContext::copyStackRemovingUnicodeEmbeddingContexts): Returns the top of a BidiContext stack that's equivalent but without contexts from Unicode directional characters. (WebCore::operator==): Now takes into account embedding source.
  • platform/text/BidiContext.h: (WebCore::BidiContext::source): Enum to specify whether an embedded bidirectional control came from the DOM/Style or Unicode characters (WebCore::BidiContext::BidiContext):
  • platform/text/BidiResolver.h: (WebCore::BidiEmbedding::BidiEmbedding): An embedding is now a direction and a hint about where it came from so we can differentiate DOM directions from unicode direction control characters. (WebCore::BidiEmbedding::direction): (WebCore::BidiEmbedding::source): (WebCore::::embed): Now takes a source as well as a direction. (WebCore::::commitExplicitEmbedding): (WebCore::::createBidiRunsForLine):
  • rendering/InlineIterator.h: (WebCore::bidiNext): (WebCore::bidiFirst):
  • rendering/RenderBlockLineLayout.cpp: (WebCore::RenderBlock::determineStartPosition):
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r82421 r82424  
     12011-03-29  Levi Weintraub  <leviw@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        RTL:  Directionality always reset on hard line break
     6        https://bugs.webkit.org/show_bug.cgi?id=23124
     7
     8        Testing that hard line breaks are being treated as paragraph separators but only
     9        clearing directional state from Unicode control characters, not DOM nodes.
     10        Also adding expected pixel results for bidi-override-in-anonymous-block as we're
     11        passing more of the test with this patch.
     12
     13        * fast/text/international/bidi-br-as-paragraph-separator.html: Added.
     14        * platform/mac/fast/css/bidi-override-in-anonymous-block-expected.checksum: Added.
     15        * platform/mac/fast/css/bidi-override-in-anonymous-block-expected.png: Added.
     16        * platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.checksum: Added.
     17        * platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.png: Added.
     18        * platform/mac/fast/text/international/bidi-br-as-paragraph-separator-expected.txt: Added.
     19
    1202011-03-30  Levi Weintraub  <leviw@chromium.org>
    221
  • trunk/Source/WebCore/ChangeLog

    r82423 r82424  
     12011-03-30  Levi Weintraub  <leviw@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        RTL:  Directionality always reset on hard line break
     6        https://bugs.webkit.org/show_bug.cgi?id=23124
     7
     8        No longer clearing all BidiContexts when we hit a hard line break.
     9        Instead, directionality applied by DOM elements is preserved by
     10        reconstructing the context stack ignoring those that didn't come
     11        from the DOM.
     12
     13        Test: fast/text/international/bidi-br-as-paragraph-separator.html
     14
     15        * platform/text/BidiContext.cpp:
     16        (WebCore::BidiContext::createUncached):
     17        (WebCore::BidiContext::create):
     18        (WebCore::copyContextAndRebaselineLevel): Helper to make a copy of a context
     19        and recalculate its bidi level.
     20        (WebCore::BidiContext::copyStackRemovingUnicodeEmbeddingContexts): Returns the top of
     21        a BidiContext stack that's equivalent but without contexts from Unicode directional
     22        characters.
     23        (WebCore::operator==): Now takes into account embedding source.
     24        * platform/text/BidiContext.h:
     25        (WebCore::BidiContext::source): Enum to specify whether an embedded
     26        bidirectional control came from the DOM/Style or Unicode characters
     27        (WebCore::BidiContext::BidiContext):
     28        * platform/text/BidiResolver.h:
     29        (WebCore::BidiEmbedding::BidiEmbedding): An embedding is now a direction
     30        and a hint about where it came from so we can differentiate DOM directions
     31        from unicode direction control characters.
     32        (WebCore::BidiEmbedding::direction):
     33        (WebCore::BidiEmbedding::source):
     34        (WebCore::::embed): Now takes a source as well as a direction.
     35        (WebCore::::commitExplicitEmbedding):
     36        (WebCore::::createBidiRunsForLine):
     37        * rendering/InlineIterator.h:
     38        (WebCore::bidiNext):
     39        (WebCore::bidiFirst):
     40        * rendering/RenderBlockLineLayout.cpp:
     41        (WebCore::RenderBlock::determineStartPosition):
     42
    1432011-03-30  Steve Block  <steveblock@google.com>
    244
  • trunk/Source/WebCore/platform/text/BidiContext.cpp

    r61324 r82424  
    2222#include "config.h"
    2323#include "BidiContext.h"
     24#include <wtf/Vector.h>
    2425
    2526namespace WebCore {
     
    2728using namespace WTF::Unicode;
    2829
    29 inline PassRefPtr<BidiContext> BidiContext::createUncached(unsigned char level, Direction direction, bool override, BidiContext* parent)
     30inline PassRefPtr<BidiContext> BidiContext::createUncached(unsigned char level, Direction direction, bool override, BidiEmbeddingSource source, BidiContext* parent)
    3031{
    31     return adoptRef(new BidiContext(level, direction, override, parent));
     32    return adoptRef(new BidiContext(level, direction, override, source, parent));
    3233}
    3334
    34 PassRefPtr<BidiContext> BidiContext::create(unsigned char level, Direction direction, bool override, BidiContext* parent)
     35PassRefPtr<BidiContext> BidiContext::create(unsigned char level, Direction direction, bool override, BidiEmbeddingSource source, BidiContext* parent)
    3536{
    3637    ASSERT(direction == (level % 2 ? RightToLeft : LeftToRight));
    3738
    3839    if (parent)
    39         return createUncached(level, direction, override, parent);
     40        return createUncached(level, direction, override, source, parent);
    4041
    4142    ASSERT(level <= 1);
    4243    if (!level) {
    4344        if (!override) {
    44             static BidiContext* ltrContext = createUncached(0, LeftToRight, false, 0).releaseRef();
     45            static BidiContext* ltrContext = createUncached(0, LeftToRight, false, FromStyleOrDOM, 0).releaseRef();
    4546            return ltrContext;
    4647        }
    4748
    48         static BidiContext* ltrOverrideContext = createUncached(0, LeftToRight, true, 0).releaseRef();
     49        static BidiContext* ltrOverrideContext = createUncached(0, LeftToRight, true, FromStyleOrDOM, 0).releaseRef();
    4950        return ltrOverrideContext;
    5051    }
    5152
    5253    if (!override) {
    53         static BidiContext* rtlContext = createUncached(1, RightToLeft, false, 0).releaseRef();
     54        static BidiContext* rtlContext = createUncached(1, RightToLeft, false, FromStyleOrDOM, 0).releaseRef();
    5455        return rtlContext;
    5556    }
    5657
    57     static BidiContext* rtlOverrideContext = createUncached(1, RightToLeft, true, 0).releaseRef();
     58    static BidiContext* rtlOverrideContext = createUncached(1, RightToLeft, true, FromStyleOrDOM, 0).releaseRef();
    5859    return rtlOverrideContext;
     60}
     61
     62static inline PassRefPtr<BidiContext> copyContextAndRebaselineLevel(BidiContext* context, BidiContext* parent)
     63{
     64    ASSERT(context);
     65    unsigned char newLevel = parent ? parent->level() : 0;
     66    if (context->dir() == RightToLeft)
     67        newLevel = nextGreaterOddLevel(newLevel);
     68    else if (parent)
     69        newLevel = nextGreaterEvenLevel(newLevel);
     70
     71    return BidiContext::create(newLevel, context->dir(), context->override(), context->source(), parent);
     72}
     73
     74// The BidiContext stack must be immutable -- they're re-used for re-layout after
     75// DOM modification/editing -- so we copy all the non-unicode contexts, and
     76// recalculate their levels.
     77PassRefPtr<BidiContext> BidiContext::copyStackRemovingUnicodeEmbeddingContexts()
     78{
     79    Vector<BidiContext*, 64> contexts;
     80    for (BidiContext* iter = this; iter; iter = iter->parent()) {
     81        if (iter->source() != FromUnicode)
     82            contexts.append(iter);
     83    }
     84    ASSERT(contexts.size());
     85 
     86    RefPtr<BidiContext> topContext = copyContextAndRebaselineLevel(contexts.last(), 0);
     87    for (int i = contexts.size() - 1; i > 0; --i)
     88        topContext = copyContextAndRebaselineLevel(contexts[i - 1], topContext.get());
     89
     90    return topContext.release();
    5991}
    6092
     
    6395    if (&c1 == &c2)
    6496        return true;
    65     if (c1.level() != c2.level() || c1.override() != c2.override() || c1.dir() != c2.dir())
     97    if (c1.level() != c2.level() || c1.override() != c2.override() || c1.dir() != c2.dir() || c1.source() != c2.source())
    6698        return false;
    6799    if (!c1.parent())
  • trunk/Source/WebCore/platform/text/BidiContext.h

    r61324 r82424  
    3131namespace WebCore {
    3232
     33enum BidiEmbeddingSource {
     34    FromStyleOrDOM,
     35    FromUnicode
     36};
     37
    3338// Used to keep track of explicit embeddings.
    3439class BidiContext : public RefCounted<BidiContext> {
    3540public:
    36     static PassRefPtr<BidiContext> create(unsigned char level, WTF::Unicode::Direction direction, bool override = false, BidiContext* parent = 0);
     41    static PassRefPtr<BidiContext> create(unsigned char level, WTF::Unicode::Direction, bool override = false, BidiEmbeddingSource = FromStyleOrDOM, BidiContext* parent = 0);
    3742
    3843    BidiContext* parent() const { return m_parent.get(); }
     
    4045    WTF::Unicode::Direction dir() const { return static_cast<WTF::Unicode::Direction>(m_direction); }
    4146    bool override() const { return m_override; }
     47    BidiEmbeddingSource source() const { return static_cast<BidiEmbeddingSource>(m_source); }
    4248
     49    PassRefPtr<BidiContext> copyStackRemovingUnicodeEmbeddingContexts();
    4350private:
    44     BidiContext(unsigned char level, WTF::Unicode::Direction direction, bool override, BidiContext* parent)
     51    BidiContext(unsigned char level, WTF::Unicode::Direction direction, bool override, BidiEmbeddingSource source, BidiContext* parent)
    4552        : m_level(level)
    4653        , m_direction(direction)
    4754        , m_override(override)
     55        , m_source(source)
    4856        , m_parent(parent)
    4957    {
    5058    }
    5159
    52     static PassRefPtr<BidiContext> createUncached(unsigned char level, WTF::Unicode::Direction, bool override, BidiContext* parent);
     60    static PassRefPtr<BidiContext> createUncached(unsigned char level, WTF::Unicode::Direction, bool override, BidiEmbeddingSource, BidiContext* parent);
    5361
    5462    unsigned char m_level;
    5563    unsigned m_direction : 5; // Direction
    5664    bool m_override : 1;
     65    unsigned m_source : 1; // BidiEmbeddingSource
    5766    RefPtr<BidiContext> m_parent;
    5867};
     68
     69inline unsigned char nextGreaterOddLevel(unsigned char level)
     70{
     71    return (level + 1) | 1;
     72}
     73
     74inline unsigned char nextGreaterEvenLevel(unsigned char level)
     75{
     76    return (level + 2) & ~1;
     77}
    5978
    6079bool operator==(const BidiContext&, const BidiContext&);
  • trunk/Source/WebCore/platform/text/BidiResolver.h

    r82395 r82424  
    7676};
    7777
     78class BidiEmbedding {
     79public:
     80    BidiEmbedding(WTF::Unicode::Direction direction, BidiEmbeddingSource source)
     81    : m_direction(direction)
     82    , m_source(source)
     83    {
     84    }
     85
     86    WTF::Unicode::Direction direction() const { return m_direction; }
     87    BidiEmbeddingSource source() const { return m_source; }
     88private:
     89    WTF::Unicode::Direction m_direction;
     90    BidiEmbeddingSource m_source;
     91};
     92
    7893inline bool operator==(const BidiStatus& status1, const BidiStatus& status2)
    7994{
     
    135150template <class Iterator, class Run> class BidiResolver {
    136151    WTF_MAKE_NONCOPYABLE(BidiResolver);
    137 public :
     152public:
    138153    BidiResolver()
    139154        : m_direction(WTF::Unicode::OtherNeutral)
     
    167182    MidpointState<Iterator>& midpointState() { return m_midpointState; }
    168183
    169     void embed(WTF::Unicode::Direction);
     184    void embed(WTF::Unicode::Direction, BidiEmbeddingSource);
    170185    bool commitExplicitEmbedding();
    171186
     
    216231    void reorderRunsFromLevels();
    217232
    218     Vector<WTF::Unicode::Direction, 8> m_currentExplicitEmbeddingSequence;
     233    Vector<BidiEmbedding, 8> m_currentExplicitEmbeddingSequence;
    219234};
    220235
     
    313328
    314329template <class Iterator, class Run>
    315 void BidiResolver<Iterator, Run>::embed(WTF::Unicode::Direction d)
     330void BidiResolver<Iterator, Run>::embed(WTF::Unicode::Direction dir, BidiEmbeddingSource source)
    316331{
    317332    using namespace WTF::Unicode;
    318333
    319     ASSERT(d == PopDirectionalFormat || d == LeftToRightEmbedding || d == LeftToRightOverride || d == RightToLeftEmbedding || d == RightToLeftOverride);
    320     m_currentExplicitEmbeddingSequence.append(d);
     334    ASSERT(dir == PopDirectionalFormat || dir == LeftToRightEmbedding || dir == LeftToRightOverride || dir == RightToLeftEmbedding || dir == RightToLeftOverride);
     335    m_currentExplicitEmbeddingSequence.append(BidiEmbedding(dir, source));
    321336}
    322337
     
    420435
    421436    for (size_t i = 0; i < m_currentExplicitEmbeddingSequence.size(); ++i) {
    422         Direction embedding = m_currentExplicitEmbeddingSequence[i];
    423         if (embedding == PopDirectionalFormat) {
     437        BidiEmbedding embedding = m_currentExplicitEmbeddingSequence[i];
     438        if (embedding.direction() == PopDirectionalFormat) {
    424439            if (BidiContext* parentContext = toContext->parent())
    425440                toContext = parentContext;
    426441        } else {
    427             Direction direction = (embedding == RightToLeftEmbedding || embedding == RightToLeftOverride) ? RightToLeft : LeftToRight;
    428             bool override = embedding == LeftToRightOverride || embedding == RightToLeftOverride;
     442            Direction direction = (embedding.direction() == RightToLeftEmbedding || embedding.direction() == RightToLeftOverride) ? RightToLeft : LeftToRight;
     443            bool override = embedding.direction() == LeftToRightOverride || embedding.direction() == RightToLeftOverride;
    429444            unsigned char level = toContext->level();
    430             if (direction == RightToLeft) {
    431                 // Go to the least greater odd integer
    432                 level += 1;
    433                 level |= 1;
    434             } else {
    435                 // Go to the least greater even integer
    436                 level += 2;
    437                 level &= ~1;
    438             }
     445            if (direction == RightToLeft)
     446                level = nextGreaterOddLevel(level);
     447            else
     448                level = nextGreaterEvenLevel(level);
    439449            if (level < 61)
    440                 toContext = BidiContext::create(level, direction, override, toContext.get());
     450                toContext = BidiContext::create(level, direction, override, embedding.source(), toContext.get());
    441451        }
    442452    }
     
    639649        if (pastEnd && (hardLineBreak || m_current.atEnd())) {
    640650            BidiContext* c = context();
    641             while (c->parent())
    642                 c = c->parent();
    643             dirCurrent = c->dir();
    644651            if (hardLineBreak) {
    645652                // A deviation from the Unicode Bidi Algorithm in order to match
    646                 // Mac OS X text and WinIE: a hard line break resets bidi state.
    647                 stateAtEnd.setContext(c);
     653                // WinIE and user expectations: hard line breaks reset bidi state
     654                // coming from unicode bidi control characters, but not those from
     655                // DOM nodes with specified directionality
     656                stateAtEnd.setContext(c->copyStackRemovingUnicodeEmbeddingContexts());
     657
     658                dirCurrent = stateAtEnd.context()->dir();
    648659                stateAtEnd.setEorDir(dirCurrent);
    649660                stateAtEnd.setLastDir(dirCurrent);
    650661                stateAtEnd.setLastStrongDir(dirCurrent);
     662            } else {
     663                while (c->parent())
     664                    c = c->parent();
     665                dirCurrent = c->dir();
    651666            }
    652667        } else {
     
    672687        case LeftToRightOverride:
    673688        case PopDirectionalFormat:
    674             embed(dirCurrent);
     689            embed(dirCurrent, FromUnicode);
    675690            commitExplicitEmbedding();
    676691            break;
  • trunk/Source/WebCore/rendering/InlineIterator.h

    r82395 r82424  
    103103    if (unicodeBidi == UBNormal)
    104104        return;
    105     resolver->embed(embedCharFromDirection(style->direction(), unicodeBidi));
     105    resolver->embed(embedCharFromDirection(style->direction(), unicodeBidi), FromStyleOrDOM);
    106106}
    107107
     
    112112    if (object->style()->unicodeBidi() == UBNormal)
    113113        return;
    114     resolver->embed(WTF::Unicode::PopDirectionalFormat);
     114    resolver->embed(WTF::Unicode::PopDirectionalFormat, FromStyleOrDOM);
    115115}
    116116
  • trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

    r82419 r82424  
    12141214        resolver.setLastDir(direction);
    12151215        resolver.setEorDir(direction);
    1216         resolver.setContext(BidiContext::create(ltr ? 0 : 1, direction, style()->unicodeBidi() == Override));
     1216        resolver.setContext(BidiContext::create(ltr ? 0 : 1, direction, style()->unicodeBidi() == Override, FromStyleOrDOM));
    12171217
    12181218        startObj = bidiFirst(this, &resolver);
Note: See TracChangeset for help on using the changeset viewer.