From 6307f3fc79929fe6acb0a83fed2fee7b4e0e3694 Mon Sep 17 00:00:00 2001 From: Simon Montagu Date: Tue, 10 Nov 2015 04:42:23 -0800 Subject: [PATCH] Bug 1216096: restore previous RTL caret behaviour by backout of bug 1164963, bug 1177505, and bug 1180417. r=jfkthame --- layout/base/nsBidiPresUtils.cpp | 37 +++++++----------- layout/base/nsCaret.cpp | 53 +++++++++----------------- layout/base/nsCaret.h | 1 - layout/base/tests/bug646382-1-ref.html | 3 +- layout/base/tests/bug646382-2-ref.html | 3 +- layout/generic/nsFrameSelection.h | 4 +- layout/generic/nsSelection.cpp | 7 ++-- layout/generic/nsTextFrame.cpp | 4 +- 8 files changed, 38 insertions(+), 74 deletions(-) diff --git a/layout/base/nsBidiPresUtils.cpp b/layout/base/nsBidiPresUtils.cpp index 18f6771fd9b..c7fdc086e62 100644 --- a/layout/base/nsBidiPresUtils.cpp +++ b/layout/base/nsBidiPresUtils.cpp @@ -734,42 +734,31 @@ nsBidiPresUtils::ResolveParagraph(nsBlockFrame* aBlockFrame, #endif #endif - nsIFrame* frame0 = frameCount > 0 ? aBpd->FrameAt(0) : nullptr; - - // Non-bidi frames - if (runCount == 1 && + if (runCount == 1 && frameCount == 1 && aBpd->mParagraphDepth == 0 && aBpd->GetDirection() == NSBIDI_LTR && - aBpd->GetParaLevel() == 0 && - frame0 && frame0 != NS_BIDI_CONTROL_FRAME && - !frame0->Properties().Get(nsIFrame::EmbeddingLevelProperty()) && - !frame0->Properties().Get(nsIFrame::BaseLevelProperty())) { - // We have a left-to-right frame in a left-to-right paragraph, + aBpd->GetParaLevel() == 0) { + // We have a single left-to-right frame in a left-to-right paragraph, // without bidi isolation from the surrounding text. - // The embedding level and base level frame properties aren't + // Make sure that the embedding level and base level frame properties aren't // set (because if they are this frame used to have some other direction, - // so we can't do this optimization) - - // Make all continuations fluid within this run - for (int i = 0; i < frameCount; ++i) { - nsIFrame* frame = aBpd->FrameAt(i); - if (frame && frame != NS_BIDI_CONTROL_FRAME) { - JoinInlineAncestors(frame); - } - } - + // so we can't do this optimization), and we're done. + nsIFrame* frame = aBpd->FrameAt(0); + if (frame != NS_BIDI_CONTROL_FRAME && + !frame->Properties().Get(nsIFrame::EmbeddingLevelProperty()) && + !frame->Properties().Get(nsIFrame::BaseLevelProperty())) { #ifdef DEBUG #ifdef NOISY_BIDI - printf("early return for single direction frame %p\n", (void*)frame); + printf("early return for single direction frame %p\n", (void*)frame); #endif #endif - - return NS_OK; + frame->AddStateBits(NS_FRAME_IS_BIDI); + return NS_OK; + } } nsIFrame* firstFrame = nullptr; nsIFrame* lastFrame = nullptr; - // Bidi frames for (; ;) { if (fragmentLength <= 0) { // Get the next frame from mLogicalFrames diff --git a/layout/base/nsCaret.cpp b/layout/base/nsCaret.cpp index 2142ef9503b..e15a068bd9c 100644 --- a/layout/base/nsCaret.cpp +++ b/layout/base/nsCaret.cpp @@ -115,14 +115,9 @@ AdjustCaretFrameForLineEnd(nsIFrame** aFrame, int32_t* aOffset) } static bool -IsKeyboardRTL() +IsBidiUI() { - bool isKeyboardRTL = false; - nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard(); - if (bidiKeyboard) { - bidiKeyboard->IsLangRTL(&isKeyboardRTL); - } - return isKeyboardRTL; + return Preferences::GetBool("bidi.browser.ui"); } nsCaret::nsCaret() @@ -503,21 +498,6 @@ nsCaret::SetCaretPosition(nsIDOMNode* aNode, int32_t aOffset) SchedulePaint(); } -bool -nsCaret::IsBidiUI() -{ - nsIFrame* frame = nullptr; - - if(Selection* selection = GetSelectionInternal()) { - int32_t contentOffset; - frame = GetFrameAndOffset(selection, mOverrideContent, mOverrideOffset, - &contentOffset); - } - - return (frame && frame->GetStateBits() & NS_FRAME_IS_BIDI) || - Preferences::GetBool("bidi.browser.ui"); -} - void nsCaret::CheckSelectionLanguageChange() { @@ -525,8 +505,11 @@ nsCaret::CheckSelectionLanguageChange() return; } - bool isKeyboardRTL = IsKeyboardRTL(); - + bool isKeyboardRTL = false; + nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard(); + if (bidiKeyboard) { + bidiKeyboard->IsLangRTL(&isKeyboardRTL); + } // Call SelectionLanguageChange on every paint. Mostly it will be a noop // but it should be fast anyway. This guarantees we never paint the caret // at the wrong place. @@ -715,9 +698,8 @@ nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection, if (theFrame->PresContext()->BidiEnabled()) { // If there has been a reflow, take the caret Bidi level to be the level of the current frame - if (aBidiLevel & BIDI_LEVEL_UNDEFINED) { + if (aBidiLevel & BIDI_LEVEL_UNDEFINED) aBidiLevel = NS_GET_EMBEDDING_LEVEL(theFrame); - } int32_t start; int32_t end; @@ -941,22 +923,21 @@ nsCaret::ComputeCaretRects(nsIFrame* aFrame, int32_t aFrameOffset, } } + // Simon -- make a hook to draw to the left or right of the caret to show keyboard language direction aHookRect->SetEmpty(); - - Selection* selection = GetSelectionInternal(); - if (!selection || !selection->GetFrameSelection()) { + if (!IsBidiUI()) { return; } - if (IsBidiUI() || IsKeyboardRTL()) { - // If caret level is RTL, draw the hook on the left; if LTR, to the right + bool isCaretRTL; + nsIBidiKeyboard* bidiKeyboard = nsContentUtils::GetBidiKeyboard(); + // if bidiKeyboard->IsLangRTL() fails, there is no way to tell the + // keyboard direction, or the user has no right-to-left keyboard + // installed, so we never draw the hook. + if (bidiKeyboard && NS_SUCCEEDED(bidiKeyboard->IsLangRTL(&isCaretRTL))) { + // If keyboard language is RTL, draw the hook on the left; if LTR, to the right // The height of the hook rectangle is the same as the width of the caret // rectangle. - int caretBidiLevel = selection->GetFrameSelection()->GetCaretBidiLevel(); - if (caretBidiLevel & BIDI_LEVEL_UNDEFINED) { - caretBidiLevel = NS_GET_EMBEDDING_LEVEL(aFrame); - } - bool isCaretRTL = caretBidiLevel % 2; if (isVertical) { bool isSidewaysLR = wm.IsVerticalLR() && !wm.IsLineInverted(); if (isSidewaysLR) { diff --git a/layout/base/nsCaret.h b/layout/base/nsCaret.h index 7f1fcc5924e..e0f84dbb225 100644 --- a/layout/base/nsCaret.h +++ b/layout/base/nsCaret.h @@ -183,7 +183,6 @@ class nsCaret final : public nsISelectionListener protected: static void CaretBlinkCallback(nsITimer *aTimer, void *aClosure); - bool IsBidiUI(); void CheckSelectionLanguageChange(); void ResetBlinking(); diff --git a/layout/base/tests/bug646382-1-ref.html b/layout/base/tests/bug646382-1-ref.html index eaa804e2e5e..03acde5b57a 100644 --- a/layout/base/tests/bug646382-1-ref.html +++ b/layout/base/tests/bug646382-1-ref.html @@ -3,14 +3,13 @@ - + diff --git a/layout/base/tests/bug646382-2-ref.html b/layout/base/tests/bug646382-2-ref.html index 6c84779cbc4..fde995d41d7 100644 --- a/layout/base/tests/bug646382-2-ref.html +++ b/layout/base/tests/bug646382-2-ref.html @@ -1,13 +1,12 @@ - + diff --git a/layout/generic/nsFrameSelection.h b/layout/generic/nsFrameSelection.h index b68f576b320..668f34b9cf8 100644 --- a/layout/generic/nsFrameSelection.h +++ b/layout/generic/nsFrameSelection.h @@ -621,9 +621,7 @@ private: uint32_t aContentOffset, nsSelectionAmount aAmount, CaretAssociateHint aHint); - void BidiLevelFromClick(nsIContent *aNewFocus, - uint32_t aContentOffset, - CaretAssociateHint aHint); + void BidiLevelFromClick(nsIContent *aNewFocus, uint32_t aContentOffset); nsPrevNextBidiLevels GetPrevNextBidiLevels(nsIContent *aNode, uint32_t aContentOffset, CaretAssociateHint aHint, diff --git a/layout/generic/nsSelection.cpp b/layout/generic/nsSelection.cpp index 543c51ce482..95604afdae1 100644 --- a/layout/generic/nsSelection.cpp +++ b/layout/generic/nsSelection.cpp @@ -1460,13 +1460,12 @@ void nsFrameSelection::BidiLevelFromMove(nsIPresShell* aPresShell, * @param aContentOffset is the new caret position, as an offset into aNode */ void nsFrameSelection::BidiLevelFromClick(nsIContent *aNode, - uint32_t aContentOffset, - CaretAssociateHint aHint) + uint32_t aContentOffset) { nsIFrame* clickInFrame=nullptr; int32_t OffsetNotUsed; - clickInFrame = GetFrameForNodeOffset(aNode, aContentOffset, aHint, &OffsetNotUsed); + clickInFrame = GetFrameForNodeOffset(aNode, aContentOffset, mHint, &OffsetNotUsed); if (!clickInFrame) return; @@ -1545,7 +1544,7 @@ nsFrameSelection::HandleClick(nsIContent* aNewFocus, // Don't take focus when dragging off of a table if (!mDragSelectingCells) { - BidiLevelFromClick(aNewFocus, aContentOffset, aHint); + BidiLevelFromClick(aNewFocus, aContentOffset); PostReason(nsISelectionListener::MOUSEDOWN_REASON + nsISelectionListener::DRAG_REASON); if (aContinueSelection && AdjustForMaintainedSelection(aNewFocus, aContentOffset)) diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 377a2dfd9cc..e3bc662254f 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -627,7 +627,7 @@ struct FlowLengthProperty { }; int32_t nsTextFrame::GetInFlowContentLength() { - if (!PresContext()->BidiEnabled()) { + if (!(mState & NS_FRAME_IS_BIDI)) { return mContent->TextLength() - mContentOffset; } @@ -639,7 +639,7 @@ int32_t nsTextFrame::GetInFlowContentLength() { * mContentOffset but this frame is empty, logically it might be before the * start of the cached flow. */ - if (flowLength && + if (flowLength && (flowLength->mStartOffset < mContentOffset || (flowLength->mStartOffset == mContentOffset && GetContentEnd() > mContentOffset)) && flowLength->mEndFlowOffset > mContentOffset) {