Attachment 555958 Details for Bug 613149

[patch] Implement bidi isolation in layout

613149.layout.2a.diff (text/plain), 17.85 KB, created by Simon Montagu :smontagu

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Montagu :smontagu

Size: 17.85 KB

patch

obsolete

># HG changeset patch
># User Simon Montagu <smontagu@smontagu.org>
># Date 1314331369 -10800
># Node ID dbd6b7fe7bb089078bfcedda94c9c666c94990b0
># Parent  d4ec0d6df5c20c4a4b12bbcdcf0d7504a55e9568
>Implement unicode-bidi: -moz-isolate. Bug 613149 try: -b do -p linux,linux64,macosx,macosx64,win32 -u all -t none
>
>diff --git a/layout/base/nsBidiPresUtils.cpp b/layout/base/nsBidiPresUtils.cpp
>--- a/layout/base/nsBidiPresUtils.cpp
>+++ b/layout/base/nsBidiPresUtils.cpp
>@@ -79,16 +79,17 @@ struct BidiParagraphData {
>   nsTArray<nsIFrame*> mLogicalFrames;
>   nsTArray<nsLineBox*> mLinePerFrame;
>   nsDataHashtable<nsISupportsHashKey, PRInt32> mContentToFrameIndex;
>   PRPackedBool        mIsVisual;
>   nsBidiLevel         mParaLevel;
>   nsIContent*         mPrevContent;
>   nsAutoPtr<nsBidi>   mBidiEngine;
>   nsIFrame*           mPrevFrame;
>+  nsAutoPtr<BidiParagraphData> mSubParagraph;
> 
>   void Init(nsBlockFrame *aBlockFrame)
>   {
>     mContentToFrameIndex.Init();
>     mBidiEngine = new nsBidi();
>     mPrevContent = nsnull;
>     mParaLevel =
>      (NS_STYLE_DIRECTION_RTL == aBlockFrame->GetStyleVisibility()->mDirection) ?
>@@ -116,16 +117,77 @@ struct BidiParagraphData {
>             content->IsXUL()) {
>           mIsVisual = PR_FALSE;
>           break;
>         }
>       }
>     }
>   }
> 
>+  BidiParagraphData* GetSubParagraph()
>+  {
>+    if (!mSubParagraph) {
>+      mSubParagraph = new BidiParagraphData();
>+      mSubParagraph->Init(this);
>+    }
>+
>+    return mSubParagraph;
>+  }
>+
>+  // Initialise a sub-paragraph from its containing paragraph
>+  void Init(BidiParagraphData *aBpd)
>+  {
>+    mContentToFrameIndex.Init();
>+    mBidiEngine = new nsBidi();
>+    mPrevContent = nsnull;
>+    mIsVisual = aBpd->mIsVisual;
>+    mParaLevel = aBpd->mParaLevel;
>+  }
>+
>+  void Reset(nsIFrame* aFrame, BidiParagraphData *aBpd)
>+  {
>+    mLogicalFrames.Clear();
>+    mLinePerFrame.Clear();
>+    mContentToFrameIndex.Clear();
>+    mBuffer.SetLength(0);
>+    mPrevFrame = aBpd->mPrevFrame;
>+    // We need to copy in embeddings (but not overrides!) from the containing
>+    // paragraph so that the line(s) including this sub-paragraph will be
>+    // correctly reordered.
>+    for (PRUint32 i = 0; i < aBpd->mEmbeddingStack.Length(); ++i) {
>+      switch(aBpd->mEmbeddingStack[i]) {
>+        case kRLE:
>+        case kRLO:
>+          mParaLevel = NextOddLevel(mParaLevel);
>+          break;
>+
>+        case kLRE:
>+        case kLRO:
>+          mParaLevel = NextEvenLevel(mParaLevel);
>+          break;
>+
>+        default:
>+          break;
>+      }
>+    }
>+
>+    nsIFrame* container = aFrame->GetParent();
>+    PRBool isRTL = (NS_STYLE_DIRECTION_RTL ==
>+                    container->GetStyleVisibility()->mDirection);
>+    if ((isRTL & 1) != (mParaLevel & 1)) {
>+      mParaLevel = isRTL ? NextOddLevel(mParaLevel) : NextEvenLevel(mParaLevel);
>+    }
>+
>+    if (container->GetStyleTextReset()->mUnicodeBidi & NS_STYLE_UNICODE_BIDI_OVERRIDE) {
>+      PushBidiControl(isRTL ? kRLO : kLRO);
>+    } else {
>+      PushBidiControl(isRTL ? kRLE : kLRE);
>+    }
>+  }
>+
>   nsresult SetPara()
>   {
>     return mBidiEngine->SetPara(mBuffer.get(), BufferLength(),
>                                 mParaLevel, nsnull);
>   }
> 
>   nsresult CountRuns(PRInt32 *runCount){ return mBidiEngine->CountRuns(runCount); }
> 
>@@ -232,16 +294,26 @@ struct BidiParagraphData {
> 
>   void ClearBidiControls()
>   {
>     for (PRUint32 i = 0; i < mEmbeddingStack.Length(); ++i) {
>       AppendUnicharFrame(NS_BIDI_CONTROL_FRAME, kPDF);
>     }
>   }
> 
>+  nsBidiLevel NextOddLevel(nsBidiLevel aLevel)
>+  {
>+    return (aLevel + 1) | 1;
>+  }
>+
>+  nsBidiLevel NextEvenLevel(nsBidiLevel aLevel)
>+  {
>+    return (aLevel + 2) & ~1;
>+  }
>+
>   static PRBool
>   IsFrameInCurrentLine(nsBlockInFlowLineIterator* aLineIter,
>                        nsIFrame* aPrevFrame, nsIFrame* aFrame)
>   {
>     nsIFrame* endFrame = aLineIter->IsLastLineInList() ? nsnull :
>       aLineIter->GetLine().next()->mFirstChild;
>     nsIFrame* startFrame = aPrevFrame ? aPrevFrame : aLineIter->GetLine()->mFirstChild;
>     for (nsIFrame* frame = startFrame; frame && frame != endFrame;
>@@ -583,18 +655,18 @@ nsBidiPresUtils::ResolveParagraph(nsBloc
>   nsIContent* content = nsnull;
>   PRInt32     contentTextLength = 0;
> 
>   FramePropertyTable *propTable = presContext->PropertyTable();
>   nsLineBox* currentLine = nsnull;
>   
> #ifdef DEBUG
> #ifdef NOISY_BIDI
>-  printf("Before Resolve(), aBlockFrame=0x%p, mBuffer='%s', frameCount=%d\n",
>-         (void*)aBlockFrame, NS_ConvertUTF16toUTF8(aBpd->mBuffer).get(), frameCount);
>+  printf("Before Resolve(), aBlockFrame=0x%p, mBuffer='%s', frameCount=%d, runCount=%d\n",
>+         (void*)aBlockFrame, NS_ConvertUTF16toUTF8(aBpd->mBuffer).get(), frameCount, runCount);
> #ifdef REALLY_NOISY_BIDI
>   printf(" block frame tree=:\n");
>   aBlockFrame->List(stdout, 0);
> #endif
> #endif
> #endif
> 
>   for (; ;) {
>@@ -794,46 +866,52 @@ PRBool IsBidiLeaf(nsIFrame* aFrame) {
>   return !kid
>     || !aFrame->IsFrameOfType(nsIFrame::eBidiInlineContainer);
> }
> 
> void
> nsBidiPresUtils::TraverseFrames(nsBlockFrame*              aBlockFrame,
>                                 nsBlockInFlowLineIterator* aLineIter,
>                                 nsIFrame*                  aCurrentFrame,
>-                                BidiParagraphData*         aBpd)
>+                                BidiParagraphData*         aBpd,
>+                                BidiParagraphData*         aContainingParagraph)
> {
>   if (!aCurrentFrame)
>     return;
> 
>   nsIFrame* childFrame = aCurrentFrame;
>   do {
>     /*
>      * It's important to get the next sibling and next continuation *before*
>      * handling the frame: If we encounter a forced paragraph break and call
>      * ResolveParagraph within this loop, doing GetNextSibling and
>      * GetNextContinuation after that could return a bidi continuation that had
>      * just been split from the original childFrame and we would process it
>      * twice.
>      */
>     nsIFrame* nextSibling = childFrame->GetNextSibling();
>     PRBool isLastFrame = !childFrame->GetNextContinuation();
>+    PRBool isFirstFrame = !childFrame->GetPrevContinuation();
> 
>     // If the real frame for a placeholder is a first letter frame, we need to
>     // drill down into it and include its contents in Bidi resolution.
>     // If not, we just use the placeholder.
>     nsIFrame* frame = childFrame;
>     if (nsGkAtoms::placeholderFrame == childFrame->GetType()) {
>       nsIFrame* realFrame =
>         nsPlaceholderFrame::GetRealFrameForPlaceholder(childFrame);
>       if (realFrame->GetType() == nsGkAtoms::letterFrame) {
>         frame = realFrame;
>       }
>     }
> 
>+    if (aContainingParagraph && isFirstFrame) {
>+      aBpd->Reset(aCurrentFrame, aContainingParagraph);
>+    }
>+
>     PRUnichar ch = 0;
>     if (frame->IsFrameOfType(nsIFrame::eBidiInlineContainer)) {
>       const nsStyleVisibility* vis = frame->GetStyleVisibility();
>       const nsStyleTextReset* text = frame->GetStyleTextReset();
>       if (text->mUnicodeBidi & NS_STYLE_UNICODE_BIDI_OVERRIDE) {
>         if (NS_STYLE_DIRECTION_RTL == vis->mDirection) {
>           ch = kRLO;
>         }
>@@ -846,17 +924,17 @@ nsBidiPresUtils::TraverseFrames(nsBlockF
>         }
>         else if (NS_STYLE_DIRECTION_LTR == vis->mDirection) {
>           ch = kLRE;
>         }
>       }
> 
>       // Add a dummy frame pointer representing a bidi control code before the
>       // first frame of an element specifying embedding or override
>-      if (ch != 0 && !frame->GetPrevContinuation()) {
>+      if (ch != 0 && isFirstFrame) {
>         aBpd->PushBidiControl(ch);
>       }
>     }
> 
>     if (IsBidiLeaf(frame)) {
>       /* Bidi leaf frame: add the frame to the mLogicalFrames array,
>        * and add its index to the mContentToFrameIndex hashtable. This
>        * will be used in RemoveBidiContinuation() to identify the last
>@@ -990,24 +1068,44 @@ nsBidiPresUtils::TraverseFrames(nsBlockF
>           // if it is not inline, end the paragraph
>           ResolveParagraphWithinBlock(aBlockFrame, aBpd);
>         }
>       }
>     }
>     else {
>       // For a non-leaf frame, recurse into TraverseFrames
>       nsIFrame* kid = frame->GetFirstPrincipalChild();
>-      TraverseFrames(aBlockFrame, aLineIter, kid, aBpd);
>+      if (kid) {
>+        const nsStyleTextReset* text = frame->GetStyleTextReset();
>+        if (text->mUnicodeBidi & NS_STYLE_UNICODE_BIDI_ISOLATE) {
>+          // css "unicode-bidi: isolate" and html5 bdi: 
>+          //  resolve the element as a separate paragraph
>+          TraverseFrames(aBlockFrame, aLineIter, kid,
>+                         aBpd->GetSubParagraph(), aBpd);
>+        } else {
>+          TraverseFrames(aBlockFrame, aLineIter, kid, aBpd);
>+        }
>+      }
>     }
> 
>     // If the element is attributed by dir, indicate direction pop (add PDF frame)
>-    if (ch != 0 && isLastFrame) {
>-      // Add a dummy frame pointer representing a bidi control code after the
>-      // last frame of an element specifying embedding or override
>-      aBpd->PopBidiControl();
>+    if (isLastFrame) {
>+      if (ch) {
>+        // Add a dummy frame pointer representing a bidi control code after the
>+        // last frame of an element specifying embedding or override
>+        aBpd->PopBidiControl();
>+      }
>+      if (aContainingParagraph) {
>+        ResolveParagraph(aBlockFrame, aBpd);
>+
>+        // Treat an element with unicode-bidi: isolate as a neutral character
>+        // within its containing paragraph
>+        aContainingParagraph->AppendUnicharFrame(NS_BIDI_CONTROL_FRAME,
>+                                                kObjectSubstitute);
>+      }
>     }
>     childFrame = nextSibling;
>   } while (childFrame);
> }
> 
> void
> nsBidiPresUtils::ResolveParagraphWithinBlock(nsBlockFrame* aBlockFrame,
>                                              BidiParagraphData* aBpd)
>diff --git a/layout/base/nsBidiPresUtils.h b/layout/base/nsBidiPresUtils.h
>--- a/layout/base/nsBidiPresUtils.h
>+++ b/layout/base/nsBidiPresUtils.h
>@@ -366,17 +366,18 @@ private:
>    *  Create a string containing the text content of all the frames
>    *  If we encounter content that requires us to split the element into more
>    *  than one paragraph for bidi resolution, resolve the paragraph up to that
>    *  point.
>    */
>   static void TraverseFrames(nsBlockFrame*              aBlockFrame,
>                              nsBlockInFlowLineIterator* aLineIter,
>                              nsIFrame*                  aCurrentFrame,
>-                             BidiParagraphData*         aBpd);
>+                             BidiParagraphData*         aBpd,
>+                             BidiParagraphData*         containingParagraph = nsnull);
>   
>   /*
>    * Position aFrame and it's descendants to their visual places. Also if aFrame
>    * is not leaf, resize it to embrace it's children.
>    *
>    * @param aFrame               The frame which itself and its children are going
>    *                             to be repositioned
>    * @param aIsOddLevel          TRUE means the embedding level of this frame is odd
>diff --git a/layout/reftests/bidi/613149-1-ref.html b/layout/reftests/bidi/613149-1-ref.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-1-ref.html
>@@ -0,0 +1,19 @@
>+<!DOCTYPE html>
>+<html>
>+ <head>
>+  <meta charset="UTF-8">
>+  <title>Bidi isolation - reference</title>
>+  <style type="text/css">
>+   body { font-family: monospace }
>+   p { unicode-bidi: bidi-override; }
>+  </style>
>+ </head>
>+ <body>
>+  <p>×××¨××§ 24 ××× ××¦××¤ - 3 reviews</p>
>+  <p>×ª×××××ª 5 - Joe's ××¤×××¤ Bar :×× ××©××¨ ×××¢×¡×</p>
>+  <p>×ª×××××ª 5 - Joe's ××¤×××¤ Bar :×× ××©××¨ ×××¢×¡×</p>
>+  <p>.(position: relative) css-× ×©××ª×©×ª</p>
>+  <p>documents &gt; ×××© ×××©××¨× ××××¨× &gt; 1 ×§×¨×¤</p>
>+  <p>joe hacker: overdrawn</p>
>+ </body>
>+</html>
>diff --git a/layout/reftests/bidi/613149-1a.html b/layout/reftests/bidi/613149-1a.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-1a.html
>@@ -0,0 +1,17 @@
>+<!DOCTYPE html>
>+<html>
>+ <head>
>+  <meta charset="UTF-8">
>+  <title>Bidi isolation - bdi element</title>
>+  
>+  <style type="text/css">body { font-family: monospace; }</style>
>+ </head>
>+ <body>
>+  <p><bdi>×¤××¦× ××× 24 ×§××¨××</bdi> - 3 reviews</p>
>+  <p>××¡×¢×× ×¨××©×× ×: <bdi dir="ltr">Joe's ×¤×××¤× Bar</bdi> - 5 ×ª×××××ª</p>
>+  <p>××¡×¢×× ×¨××©×× ×: <bdi dir="ltr"><bdo dir="rtl">raB ×¤×××¤× s'eoJ</bdo></bdi> - 5 ×ª×××××ª</p>
>+  <p dir="rtl" align="left">×ª×©×ª××© ×-css (<bdi dir="ltr">position: relative</bdi>).</p>
>+  <p>documents &gt; <bdi dir="rtl">××¨××× ××¨××©×× ×©××</bdi> &gt; <bdi dir="rtl">×¤×¨×§ 1</bdi></p>
>+  <p><bdi>joe hacker&#x202e;</bdi>: overdrawn</p>
>+ </body>
>+</html>
>diff --git a/layout/reftests/bidi/613149-1b.html b/layout/reftests/bidi/613149-1b.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-1b.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+ <head>
>+  <meta charset="UTF-8">
>+  <title>Bidi isolation - css</title>
>+  <style type="text/css">
>+   body { font-family: monospace; }
>+   span { unicode-bidi: -moz-isolate; }
>+   span.override { unicode-bidi: bidi-override -moz-isolate; }
>+  </style>
>+ </head>
>+ <body>
>+  <p><span>×¤××¦× ××× 24 ×§××¨××</span> - 3 reviews</p>
>+  <p>××¡×¢×× ×¨××©×× ×: <span dir="ltr">Joe's ×¤×××¤× Bar</span> - 5 ×ª×××××ª</p>
>+  <p>××¡×¢×× ×¨××©×× ×: <span class="override" dir="rtl">raB ×¤×××¤× s'eoJ</span> - 5 ×ª×××××ª</p>
>+  <p dir="rtl" align="left">×ª×©×ª××© ×-css (<span dir="ltr">position: relative</span>).</p>
>+  <p>documents &gt; <span dir="rtl">××¨××× ××¨××©×× ×©××</span> &gt; <span dir="rtl">×¤×¨×§ 1</span></p>
>+  <p><span>joe hacker&#x202e;</span>: overdrawn</p>
>+ </body>
>+</html>
>diff --git a/layout/reftests/bidi/613149-2-ref.html b/layout/reftests/bidi/613149-2-ref.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-2-ref.html
>@@ -0,0 +1,9 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
>+    <title>Google Reader minimized test case</title>
>+  </head>
>+  <body>
>+    <div style="unicode-bidi: bidi-override; text-align: right">××¡×¨× ×××××¢ ×¨××× ×ª××××× ×××¢× :Mozilla Firefox â¢ Re</div>
>+</body></html>
>diff --git a/layout/reftests/bidi/613149-2a.html b/layout/reftests/bidi/613149-2a.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-2a.html
>@@ -0,0 +1,12 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
>+    <title>Google Reader minimized test case</title>
>+    <style type="text/css">
>+     span div {display: inline;}
>+    </style>
>+  </head>
>+  <body>
>+    <div dir="rtl"><span><div>Mozilla Firefox â¢ Re: ××¢×× ××××××ª ××××¨ ×¢×××× ××¨×¡×</div></span></div>
>+</body></html>
>diff --git a/layout/reftests/bidi/613149-2b.html b/layout/reftests/bidi/613149-2b.html
>new file mode 100644
>--- /dev/null
>+++ b/layout/reftests/bidi/613149-2b.html
>@@ -0,0 +1,18 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
>+    <title>Google Reader minimized test case</title>
>+    <style type="text/css">
>+     span div {display: inline;}
>+    </style>
>+    <script type="text/javascript">
>+function boom()
>+{
>+  document.getElementById("d").style.fontWeight="normal";
>+}
>+    </script>
>+  </head>
>+  <body onload="boom()">
>+    <div dir="rtl"><span><div id="d" style="font-weight: bold;">Mozilla Firefox â¢ Re: ××¢×× ××××××ª ××××¨ ×¢×××× ××¨×¡×</div></span></div>
>+</body></html>
>diff --git a/layout/reftests/bidi/reftest.list b/layout/reftests/bidi/reftest.list
>--- a/layout/reftests/bidi/reftest.list
>+++ b/layout/reftests/bidi/reftest.list
>@@ -75,11 +75,15 @@ fails-if(Android) == 386339.html 386339-
> == 503269-1.html 503269-1-ref.html
> == 503957-1.html 503957-1-ref.html
> == 525740-1.html 525740-1-ref.html
> == 536963-1.html 536963-1-ref.html
> == 588739-1.html 588739-ref.html
> == 588739-2.html 588739-ref.html
> == 588739-3.html 588739-ref.html
> == 612843-1.html 612843-1-ref.html
>+== 613149-1a.html 613149-1-ref.html
>+== 613149-1b.html 613149-1-ref.html
>+== 613149-2a.html 613149-2-ref.html
>+== 613149-2b.html 613149-2-ref.html
> == 613157-1.html 613157-1-ref.html
> == 613157-2.html 613157-2-ref.html
> == 670226-1.html 670226-1-ref.html
>diff --git a/layout/style/html.css b/layout/style/html.css
>--- a/layout/style/html.css
>+++ b/layout/style/html.css
>@@ -43,17 +43,20 @@
> [dir="rtl"] {
>   direction: rtl;
>   unicode-bidi: embed;
> }
> [dir="ltr"] {
>   direction: ltr;
>   unicode-bidi: embed;
> }
>-bdo[dir] {
>+bdi, bdi[dir], output, output[dir] { 
>+  unicode-bidi: -moz-isolate;
>+}
>+bdo, bdo[dir] {
>   unicode-bidi: bidi-override;
> }
> 
> /* To ensure http://www.w3.org/TR/REC-html40/struct/dirlang.html#style-bidi:
>  *
>  * "When a block element that does not have a dir attribute is transformed to
>  * the style of an inline element by a style sheet, the resulting presentation
>  * should be equivalent, in terms of bidirectional formatting, to the
>@@ -103,17 +106,17 @@ table,
> tbody,
> td,
> tfoot,
> th,
> thead,
> tr,
> ul,
> xmp {
>-  unicode-bidi: embed;
>+  unicode-bidi: -moz-isolate;
> }
> 
> 
> /* blocks */
> 
> article,
> aside,
> div,
>@@ -633,17 +636,17 @@ hr[size="1"] {
> }
> 
> img:-moz-broken::before, input:-moz-broken::before,
> img:-moz-user-disabled::before, input:-moz-user-disabled::before,
> img:-moz-loading::before, input:-moz-loading::before,
> applet:-moz-empty-except-children-with-localname(param):-moz-broken::before,
> applet:-moz-empty-except-children-with-localname(param):-moz-user-disabled::before {
>   content: -moz-alt-content !important;
>-  unicode-bidi: embed;
>+  unicode-bidi: -moz-isolate;
> }
> 
> :-moz-any(object,applet):-moz-any(:-moz-broken,:-moz-user-disabled) > *|* {
>   /*
>     Inherit in the object's alignment so that if we aren't aligned explicitly
>     we'll end up in the right place vertically.  See bug 36997.  Note that this
>     is not !important because we _might_ be aligned explicitly.
>   */

Flags:

roc: review+

Actions: View | Diff | Review

Attachments on bug 613149: 525110 | 525114 | 525116 | 536698 | 537430 | 555742 | 555776 | 555958 | 555962