Bug 1048752. Part 22: Factor out SelectionLanguageChange call to run early in nsCaret::GetPaintGeometry. r=tn

This code is somewhat tricky. nsCaret::ComputeCaretRects was checking to see
if we have a bidi keyboard, and if so, what direction it's set to.
If the direction changed from the last direction seen for *this caret*,
we fired a SelectionLanguageChange notification on the caret's current
Selection. This looked bogus because the caret can be switched between
selections so it would seem some selections won't get a notification when
they should, but that's how it was. Also, when the SelectionLanguageChange
notification fired we then didn't draw the caret in that iteration, which
seems even more bogus.

This patch fixes all that by moving the logic to fire SelectionLanguageChange
out to GetPaintGeometry and firing the notification every single time without
trying to detect whether the state has changed or not. I carefully examined
the implementation of SelectionLanguageChange and I'm pretty sure it's
idempotent so this should be correct. That doesn't look like an
expensive function, and runs at most once per window paint, so I'm not
worried about perf. Because we now fire SelectionLanguageChange before
reading selection or frame state, it should be fine to carry on after
calling SelectionLanguageChange and drawing the caret based on whatever
changes SelectionLanguageChange has performed.

This also lets us remove mKeyboardRTL, which as noted above seems inherently
bogus.

--HG--
extra : rebase_source : 3ddfd10f6f30033e090e72b4bb43f2695218752e
This commit is contained in:
Robert O'Callahan 2014-08-06 17:19:28 +12:00
parent ad25f90af2
commit b1e6bddb83
2 changed files with 34 additions and 32 deletions

View File

@ -108,6 +108,12 @@ AdjustCaretFrameForLineEnd(nsIFrame** aFrame, int32_t* aOffset)
}
}
static bool
IsBidiUI()
{
return Preferences::GetBool("bidi.browser.ui");
}
//-----------------------------------------------------------------------------
nsCaret::nsCaret()
@ -119,7 +125,6 @@ nsCaret::nsCaret()
, mReadOnly(false)
, mShowDuringSelection(false)
, mIgnoreUserModify(true)
, mKeyboardRTL(false)
, mLastBidiLevel(0)
, mLastContentOffset(0)
, mLastHint(CARET_ASSOCIATE_BEFORE)
@ -449,6 +454,27 @@ nsresult nsCaret::DrawAtPosition(nsIDOMNode* aNode, int32_t aOffset)
return rv;
}
void
nsCaret::CheckSelectionLanguageChange()
{
// Simon -- make a hook to draw to the left or right of the caret to show keyboard language direction
bool isKeyboardRTL = false;
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(&isKeyboardRTL)) &&
IsBidiUI()) {
// 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.
Selection* selection = GetSelectionInternal();
if (selection) {
selection->SelectionLanguageChange(isKeyboardRTL);
}
}
}
nsIFrame*
nsCaret::GetPaintGeometry(nsRect* aRect)
{
@ -456,6 +482,10 @@ nsCaret::GetPaintGeometry(nsRect* aRect)
if (!mDrawn)
return nullptr;
// Update selection language direction now so the new direction will be
// taken into account when computing the caret position below.
CheckSelectionLanguageChange();
nsFrameSelection* frameSelection = GetFrameSelection();
if (!frameSelection)
return nullptr;
@ -688,12 +718,6 @@ nsCaret::DrawAtPositionWithHint(nsIDOMNode* aNode,
return true;
}
static bool
IsBidiUI()
{
return Preferences::GetBool("bidi.browser.ui");
}
nsresult
nsCaret::GetCaretFrameForNodeOffset(nsFrameSelection* aFrameSelection,
nsIContent* aContentNode,
@ -1084,36 +1108,14 @@ nsCaret::ComputeCaretRects(nsIFrame* aFrame, int32_t aFrameOffset,
aHookRect->SetEmpty();
// Simon -- make a hook to draw to the left or right of the caret to show keyboard language direction
bool isCaretRTL = false;
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)) &&
IsBidiUI()) {
if (isCaretRTL != mKeyboardRTL) {
/* if the caret bidi level and the keyboard language direction are not in
* synch, the keyboard language must have been changed by the
* user, and if the caret is in a boundary condition (between left-to-right and
* right-to-left characters) it may have to change position to
* reflect the location in which the next character typed will
* appear. We will call |SelectionLanguageChange| and exit
* without drawing the caret in the old position.
*/
mKeyboardRTL = isCaretRTL;
nsCOMPtr<nsISelectionPrivate> domSelection = do_QueryReferent(mDomSelectionWeak);
if (!domSelection ||
NS_SUCCEEDED(domSelection->SelectionLanguageChange(mKeyboardRTL)))
*aCaretRect = *aHookRect = nsRect();
return;
}
// 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.
aHookRect->SetRect(aCaretRect->x + ((isCaretRTL) ?
bidiIndicatorSize * -1 :
aCaretRect->width),
aHookRect->SetRect(aCaretRect->x + (isCaretRTL ? bidiIndicatorSize * -1 : aCaretRect->width),
aCaretRect->y + bidiIndicatorSize,
bidiIndicatorSize,
aCaretRect->width);

View File

@ -161,6 +161,7 @@ protected:
// Schedule a repaint for the frame where the caret would appear.
// Does not check visibility etc.
void SchedulePaint();
void CheckSelectionLanguageChange();
void KillTimer();
nsresult PrimeTimer();
@ -229,7 +230,6 @@ protected:
bool mIgnoreUserModify;
bool mKeyboardRTL; // is the keyboard language right-to-left
uint8_t mLastBidiLevel; // saved bidi level of the last draw request, to use when we erase
nsCOMPtr<nsIContent> mLastContent; // store the content the caret was last requested to be drawn