Attachment 498345 Details for Bug 263359

[patch] Patch part 2 v3: more refactoring -- spin ResolveParagraph out of Resolve; updated to comments

607541.1b.diff (text/plain), 11.35 KB, created by Simon Montagu :smontagu

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Montagu :smontagu

Size: 11.35 KB

patch

obsolete

>diff --git a/layout/base/nsBidiPresUtils.cpp b/layout/base/nsBidiPresUtils.cpp
>--- a/layout/base/nsBidiPresUtils.cpp
>+++ b/layout/base/nsBidiPresUtils.cpp
>@@ -310,21 +310,59 @@ nsresult
> nsBidiPresUtils::Resolve(nsBlockFrame* aBlockFrame)
> {
>   mLogicalFrames.Clear();
>   mContentToFrameIndex.Clear();
>   mBuffer.SetLength(0);
>   
>   nsPresContext *presContext = aBlockFrame->PresContext();
> 
>+  const nsStyleVisibility* vis = aBlockFrame->GetStyleVisibility();
>+
>+  mParaLevel = (NS_STYLE_DIRECTION_RTL == vis->mDirection) ?
>+                NSBIDI_RTL : NSBIDI_LTR;
>+
>+  mLineIter = new nsBlockInFlowLineIterator(aBlockFrame,
>+                                            aBlockFrame->begin_lines(),
>+                                            PR_FALSE);
>+  if (mLineIter->GetLine() == aBlockFrame->end_lines()) {
>+    // Advance to first valid line (might be in a next-continuation)
>+    mLineIter->Next();
>+  }
>+
>+  mIsVisual = presContext->IsVisualMode();
>+  if (mIsVisual) {
>+    /**
>+     * Drill up in content to detect whether this is an element that needs to be
>+     * rendered with logical order even on visual pages.
>+     *
>+     * We always use logical order on form controls, firstly so that text entry
>+     * will be in logical order, but also because visual pages were written with
>+     * the assumption that even if the browser had no support for right-to-left
>+     * text rendering, it would use native widgets with bidi support to display
>+     * form controls.
>+     *
>+     * We also use logical order in XUL elements, since we expect that if a XUL
>+     * element appears in a visual page, it will be generated by an XBL binding
>+     * and contain localized text which will be in logical order.
>+     */
>+    for (nsIContent* content = aBlockFrame->GetContent() ; content; 
>+         content = content->GetParent()) {
>+      if (content->IsNodeOfType(nsINode::eHTML_FORM_CONTROL) ||
>+          content->IsXUL()) {
>+        mIsVisual = PR_FALSE;
>+        break;
>+      }
>+    }
>+  }
>+  mPrevFrame = nsnull;
>+
>   // handle bidi-override being set on the block itself before calling
>   // InitLogicalArray.
>-  const nsStyleVisibility* vis = aBlockFrame->GetStyleVisibility();
>   const nsStyleTextReset* text = aBlockFrame->GetStyleTextReset();
>-
>   PRUnichar ch = 0;
>   if (text->mUnicodeBidi == NS_STYLE_UNICODE_BIDI_OVERRIDE) {
>     if (NS_STYLE_DIRECTION_RTL == vis->mDirection) {
>       ch = kRLO;
>     }
>     else if (NS_STYLE_DIRECTION_LTR == vis->mDirection) {
>       ch = kLRO;
>     }
>@@ -342,85 +380,57 @@ nsBidiPresUtils::Resolve(nsBlockFrame* a
> 
>   if (ch != 0) {
>     mLogicalFrames.AppendElement(NS_BIDI_CONTROL_FRAME);
>     mBuffer.Append(kPDF);
>   }
> 
>   // XXX: TODO: Handle preformatted text ('\n')
>   mBuffer.ReplaceChar("\t\r\n", kSpace);
>+  ResolveParagraph(aBlockFrame);
>+  return mSuccess;
>+}
> 
>+void
>+nsBidiPresUtils::ResolveParagraph(nsBlockFrame* aBlockFrame)
>+{
>+  nsPresContext *presContext = aBlockFrame->PresContext();
>   PRInt32 bufferLength = mBuffer.Length();
> 
>   if (bufferLength < 1) {
>     mSuccess = NS_OK;
>-    return mSuccess;
>+    return;
>   }
>   PRInt32 runCount;
>-  PRUint8 embeddingLevel;
>+  PRUint8 embeddingLevel = mParaLevel;
> 
>-  nsBidiLevel paraLevel = embeddingLevel =
>-    (NS_STYLE_DIRECTION_RTL == vis->mDirection)
>-    ? NSBIDI_RTL : NSBIDI_LTR;
>-
>-  mSuccess = mBidiEngine->SetPara(mBuffer.get(), bufferLength, paraLevel, nsnull);
>+  mSuccess = mBidiEngine->SetPara(mBuffer.get(), bufferLength, mParaLevel, nsnull);
>   if (NS_FAILED(mSuccess) ) {
>-      return mSuccess;
>+      return;
>   }
> 
>   mSuccess = mBidiEngine->CountRuns(&runCount);
>   if (NS_FAILED(mSuccess) ) {
>-    return mSuccess;
>+    return;
>   }
>   PRInt32     runLength      = 0;   // the length of the current run of text
>   PRInt32     lineOffset     = 0;   // the start of the current run
>   PRInt32     logicalLimit   = 0;   // the end of the current run + 1
>   PRInt32     numRun         = -1;
>   PRInt32     fragmentLength = 0;   // the length of the current text frame
>   PRInt32     frameIndex     = -1;  // index to the frames in mLogicalFrames
>   PRInt32     frameCount     = mLogicalFrames.Length();
>   PRInt32     contentOffset  = 0;   // offset of current frame in its content node
>   PRBool      isTextFrame    = PR_FALSE;
>   nsIFrame*   frame = nsnull;
>   nsIContent* content = nsnull;
>   PRInt32     contentTextLength;
> 
>   FramePropertyTable *propTable = presContext->PropertyTable();
>-
>-  nsBlockInFlowLineIterator lineIter(aBlockFrame, aBlockFrame->begin_lines(), PR_FALSE);
>-  if (lineIter.GetLine() == aBlockFrame->end_lines()) {
>-    // Advance to first valid line (might be in a next-continuation)
>-    lineIter.Next();
>-  }
>-  nsIFrame* prevFrame = nsnull;
>   PRBool lineNeedsUpdate = PR_FALSE;
>-
>-  PRBool isVisual = presContext->IsVisualMode();
>-  if (isVisual) {
>-    /**
>-     * Drill up in content to detect whether this is an element that needs to be
>-     * rendered with logical order even on visual pages.
>-     *
>-     * We always use logical order on form controls, firstly so that text entry
>-     * will be in logical order, but also because visual pages were written with
>-     * the assumption that even if the browser had no support for right-to-left
>-     * text rendering, it would use native widgets with bidi support to display
>-     * form controls.
>-     *
>-     * We also use logical order in XUL elements, since we expect that if a XUL
>-     * element appears in a visual page, it will be generated by an XBL binding
>-     * and contain localized text which will be in logical order.
>-     */
>-    for (content = aBlockFrame->GetContent() ; content; content = content->GetParent()) {
>-      if (content->IsNodeOfType(nsINode::eHTML_FORM_CONTROL) || content->IsXUL()) {
>-        isVisual = PR_FALSE;
>-        break;
>-      }
>-    }
>-  }
>   
> #ifdef NOISY_BIDI
>   if (mBuffer[0] != kObjectSubstitute) {
>     printf("Before Resolve(), aBlockFrame=0x%p, mBuffer='%s', frameCount=%d\n",
>            (void*)aBlockFrame, NS_ConvertUTF16toUTF8(mBuffer).get(), frameCount);
> #ifdef REALLY_NOISY_BIDI
>     printf(" frameTree=:\n");
>     nsFrame::DumpFrameTree(aBlockFrame);
>@@ -453,17 +463,17 @@ nsBidiPresUtils::Resolve(nsBlockFrame* a
>         contentTextLength = content->TextLength();
>         if (contentTextLength == 0) {
>           frame->AdjustOffsetsForBidi(0, 0);
>           // Set the base level and embedding level of the current run even
>           // on an empty frame. Otherwise frame reordering will not be correct.
>           propTable->Set(frame, nsIFrame::EmbeddingLevelProperty(),
>                          NS_INT32_TO_PTR(embeddingLevel));
>           propTable->Set(frame, nsIFrame::BaseLevelProperty(),
>-                         NS_INT32_TO_PTR(paraLevel));
>+                         NS_INT32_TO_PTR(mParaLevel));
>           continue;
>         }
>         PRInt32 start, end;
>         frame->GetOffsets(start, end);
>         NS_ASSERTION(!(contentTextLength < end - start),
>                      "Frame offsets don't fit in content");
>         fragmentLength = NS_MIN(contentTextLength, end - start);
>         contentOffset = start;
>@@ -477,41 +487,41 @@ nsBidiPresUtils::Resolve(nsBlockFrame* a
>         break;
>       }
>       lineOffset = logicalLimit;
>       if (NS_FAILED(mBidiEngine->GetLogicalRun(
>               lineOffset, &logicalLimit, &embeddingLevel) ) ) {
>         break;
>       }
>       runLength = logicalLimit - lineOffset;
>-      if (isVisual) {
>-        embeddingLevel = paraLevel;
>+      if (mIsVisual) {
>+        embeddingLevel = mParaLevel;
>       }
>     } // if (runLength <= 0)
> 
>     if (frame == NS_BIDI_CONTROL_FRAME) {
>       frame = nsnull;
>       ++lineOffset;
>     }
>     else {
>       propTable->Set(frame, nsIFrame::EmbeddingLevelProperty(),
>                      NS_INT32_TO_PTR(embeddingLevel));
>       propTable->Set(frame, nsIFrame::BaseLevelProperty(),
>-                     NS_INT32_TO_PTR(paraLevel));
>+                     NS_INT32_TO_PTR(mParaLevel));
>       if (isTextFrame) {
>         if ( (runLength > 0) && (runLength < fragmentLength) ) {
>           /*
>            * The text in this frame continues beyond the end of this directional run.
>            * Create a non-fluid continuation frame for the next directional run.
>            */
>           if (lineNeedsUpdate) {
>-            AdvanceLineIteratorToFrame(frame, &lineIter, prevFrame);
>+            AdvanceLineIteratorToFrame(frame, mLineIter, mPrevFrame);
>             lineNeedsUpdate = PR_FALSE;
>           }
>-          lineIter.GetLine()->MarkDirty();
>+          mLineIter->GetLine()->MarkDirty();
>           nsIFrame* nextBidi;
>           PRInt32 runEnd = contentOffset + runLength;
>           EnsureBidiContinuation(frame, &nextBidi, frameIndex,
>                                  contentOffset,
>                                  runEnd);
>           if (NS_FAILED(mSuccess)) {
>             break;
>           }
>@@ -552,20 +562,20 @@ nsBidiPresUtils::Resolve(nsBlockFrame* a
>             nsIFrame* next = frame->GetNextInFlow();
>             if (next) {
>               frame->SetNextContinuation(next);
>               next->SetPrevContinuation(frame);
>             }
>           }
>           frame->AdjustOffsetsForBidi(contentOffset, contentOffset + fragmentLength);
>           if (lineNeedsUpdate) {
>-            AdvanceLineIteratorToFrame(frame, &lineIter, prevFrame);
>+            AdvanceLineIteratorToFrame(frame, mLineIter, mPrevFrame);
>             lineNeedsUpdate = PR_FALSE;
>           }
>-          lineIter.GetLine()->MarkDirty();
>+          mLineIter->GetLine()->MarkDirty();
>         }
>       } // isTextFrame
>       else {
>         ++lineOffset;
>       }
>     } // not bidi control frame
>     PRInt32 temp = runLength;
>     runLength -= fragmentLength;
>@@ -611,17 +621,16 @@ nsBidiPresUtils::Resolve(nsBlockFrame* a
>   } // for
> #ifdef REALLY_NOISY_BIDI
>   if (mBuffer[0] != kObjectSubstitute) {
>     printf("---\nAfter Resolve(), frameTree =:\n");
>     nsFrame::DumpFrameTree(aBlockFrame);
>     printf("===\n");
>   }
> #endif
>-  return mSuccess;
> }
> 
> // Should this frame be treated as a leaf (e.g. when building mLogicalFrames)?
> PRBool IsBidiLeaf(nsIFrame* aFrame) {
>   nsIFrame* kid = aFrame->GetFirstChild(nsnull);
>   return !kid
>     || !aFrame->IsFrameOfType(nsIFrame::eBidiInlineContainer);
> }
>diff --git a/layout/base/nsBidiPresUtils.h b/layout/base/nsBidiPresUtils.h
>--- a/layout/base/nsBidiPresUtils.h
>+++ b/layout/base/nsBidiPresUtils.h
>@@ -170,16 +170,17 @@ public:
>    * Make Bidi engine calculate the embedding levels of the frames that are
>    * descendants of a given block frame.
>    *
>    * @param aBlockFrame          The block frame
>    *
>    *  @lina 06/18/2000
>    */
>   nsresult Resolve(nsBlockFrame* aBlockFrame);
>+  void ResolveParagraph(nsBlockFrame* aBlockFrame);
> 
>   /**
>    * Reorder this line using Bidi engine.
>    * Update frame array, following the new visual sequence.
>    * 
>    * @lina 05/02/2000
>    */
>   void ReorderFrames(nsIFrame*            aFirstFrameOnLine,
>@@ -473,16 +474,20 @@ private:
>   nsAutoString    mBuffer;
>   nsTArray<nsIFrame*> mLogicalFrames;
>   nsTArray<nsIFrame*> mVisualFrames;
>   nsDataHashtable<nsISupportsHashKey, PRInt32> mContentToFrameIndex;
>   PRInt32         mArraySize;
>   PRInt32*        mIndexMap;
>   PRUint8*        mLevels;
>   nsresult        mSuccess;
>+  PRPackedBool    mIsVisual;
>+  nsBidiLevel     mParaLevel;
>+  nsIFrame*       mPrevFrame;
>   nsIContent*     mPrevContent;
> 
>+  nsAutoPtr<nsBlockInFlowLineIterator> mLineIter;
>   nsBidi*         mBidiEngine;
> };
> 
> #endif /* nsBidiPresUtils_h___ */
> 
> #endif // IBMBIDI

Flags:

smontagu: review+

Actions: View | Diff | Review

Attachments on bug 263359: 161392 | 161393 | 161394 | 161455 | 493937 | 493938 | 493939 | 493940 | 495867 | 495868 | 495869 | 495870 | 495873 | 495874 | 498344 | 498345 | 498346 | 498347 | 498349 | 498351 | 498356 | 498455 | 521980 | 522787 | 522788 | 537515