From d0a0fb4fcf398e2418944090aa8ad6d6139dd6c4 Mon Sep 17 00:00:00 2001 From: Mike de Boer Date: Wed, 9 Nov 2016 15:09:36 -0800 Subject: [PATCH] Bug 1302470 Part 2: Use a hit-test method to determine if the rect of a range is visible on the page or not to the eye, for use in find-in-page. r=mstange,smaug MozReview-Commit-ID: 9P7gf0GcREv --- layout/base/nsLayoutUtils.h | 2 +- .../typeaheadfind/nsITypeAheadFind.idl | 4 +- .../typeaheadfind/nsTypeAheadFind.cpp | 127 +++++------------- .../typeaheadfind/nsTypeAheadFind.h | 4 +- toolkit/modules/FinderIterator.jsm | 3 +- 5 files changed, 37 insertions(+), 103 deletions(-) diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index 40385d255e97..39a566d23384 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -825,7 +825,7 @@ public: * frames under the area of a rectangle that receives a mouse event, * or nullptr if there is no such frame. * @param aRect the rect, relative to the frame origin - * @param aOutFrames an array to add all the frames found + * @param aOutFrames an array to append all the frames found * @param aFlags some combination of FrameForPointFlags */ static nsresult GetFramesForArea(nsIFrame* aFrame, const nsRect& aRect, diff --git a/toolkit/components/typeaheadfind/nsITypeAheadFind.idl b/toolkit/components/typeaheadfind/nsITypeAheadFind.idl index 379d2c2a261e..0056d0d21944 100644 --- a/toolkit/components/typeaheadfind/nsITypeAheadFind.idl +++ b/toolkit/components/typeaheadfind/nsITypeAheadFind.idl @@ -18,7 +18,7 @@ interface nsIDocShell; /****************************** nsTypeAheadFind ******************************/ -[scriptable, uuid(ae501e28-c57f-4692-ac74-410e1bed98b7)] +[scriptable, uuid(3cfe7906-f189-45a0-8abe-8e4437a23cae)] interface nsITypeAheadFind : nsISupports { /****************************** Initializer ******************************/ @@ -58,7 +58,7 @@ interface nsITypeAheadFind : nsISupports void collapseSelection(); /* Check if a range is visible */ - boolean isRangeVisible(in nsIDOMRange aRange, in boolean aMustBeInViewPort); + boolean isRangeVisible(in nsIDOMRange aRange, in boolean aFlushLayout); /******************************* Attributes ******************************/ diff --git a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp index f4532150026f..7dd24233905e 100644 --- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp +++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp @@ -52,6 +52,7 @@ #include "mozilla/dom/Link.h" #include "nsRange.h" #include "nsXBLBinding.h" +#include "nsLayoutUtils.h" #include "nsTypeAheadFind.h" @@ -70,8 +71,6 @@ NS_IMPL_CYCLE_COLLECTION(nsTypeAheadFind, mFoundLink, mFoundEditable, mStartPointRange, mEndPointRange, mSoundInterface, mFind, mFoundRange) -static NS_DEFINE_CID(kFrameTraversalCID, NS_FRAMETRAVERSAL_CID); - #define NS_FIND_CONTRACTID "@mozilla.org/embedcomp/rangefind;1" nsTypeAheadFind::nsTypeAheadFind(): @@ -436,8 +435,7 @@ nsTypeAheadFind::FindItNow(nsIPresShell *aPresShell, bool aIsLinksOnly, } bool usesIndependentSelection; - if (!IsRangeVisible(presShell, presContext, returnRange, - aIsFirstVisiblePreferred, false, + if (!IsRangeVisible(presShell, presContext, returnRange, true, getter_AddRefs(mStartPointRange), &usesIndependentSelection) || (aIsLinksOnly && !isInsideLink) || @@ -817,8 +815,7 @@ nsTypeAheadFind::GetSearchContainers(nsISupports *aContainer, // Ensure visible range, move forward if necessary // This uses ignores the return value, but usese the side effect of // IsRangeVisible. It returns the first visible range after searchRange - IsRangeVisible(presShell, presContext, mSearchRange, - aIsFirstVisiblePreferred, true, + IsRangeVisible(presShell, presContext, mSearchRange, true, getter_AddRefs(mStartPointRange), nullptr); } else { @@ -1161,7 +1158,7 @@ nsTypeAheadFind::GetFoundRange(nsIDOMRange** aFoundRange) NS_IMETHODIMP nsTypeAheadFind::IsRangeVisible(nsIDOMRange *aRange, - bool aMustBeInViewPort, + bool aFlushLayout, bool *aResult) { // Jump through hoops to extract the docShell from the range. @@ -1178,8 +1175,7 @@ nsTypeAheadFind::IsRangeVisible(nsIDOMRange *aRange, nsCOMPtr presShell (docShell->GetPresShell()); RefPtr presContext = presShell->GetPresContext(); nsCOMPtr startPointRange = new nsRange(presShell->GetDocument()); - *aResult = IsRangeVisible(presShell, presContext, aRange, - aMustBeInViewPort, false, + *aResult = IsRangeVisible(presShell, presContext, aRange, aFlushLayout, getter_AddRefs(startPointRange), nullptr); return NS_OK; @@ -1188,8 +1184,8 @@ nsTypeAheadFind::IsRangeVisible(nsIDOMRange *aRange, bool nsTypeAheadFind::IsRangeVisible(nsIPresShell *aPresShell, nsPresContext *aPresContext, - nsIDOMRange *aRange, bool aMustBeInViewPort, - bool aGetTopVisibleLeaf, + nsIDOMRange *aRange, + bool aFlushLayout, nsIDOMRange **aFirstVisibleRange, bool *aUsesIndependentSelection) { @@ -1199,8 +1195,12 @@ nsTypeAheadFind::IsRangeVisible(nsIPresShell *aPresShell, // We need to know if the range start is visible. // Otherwise, return the first visible range start // in aFirstVisibleRange - aRange->CloneRange(aFirstVisibleRange); + + if (aFlushLayout) { + aPresShell->FlushPendingNotifications(Flush_Layout); + } + nsCOMPtr node; aRange->GetStartContainer(getter_AddRefs(node)); @@ -1209,10 +1209,25 @@ nsTypeAheadFind::IsRangeVisible(nsIPresShell *aPresShell, return false; nsIFrame *frame = content->GetPrimaryFrame(); - if (!frame) + if (!frame) return false; // No frame! Not visible then. - if (!frame->StyleVisibility()->IsVisible()) + // Having a primary frame doesn't mean that the range is visible inside the + // viewport. Do a hit-test to determine that quickly and properly. + AutoTArray frames; + nsIFrame *rootFrame = aPresShell->GetRootFrame(); + RefPtr range = static_cast(aRange); + RefPtr rects = range->GetClientRects(true, false); + for (uint32_t i = 0; i < rects->Length(); ++i) { + RefPtr rect = rects->Item(i); + nsRect r(nsPresContext::CSSPixelsToAppUnits((float)rect->X()), + nsPresContext::CSSPixelsToAppUnits((float)rect->Y()), + nsPresContext::CSSPixelsToAppUnits((float)rect->Width()), + nsPresContext::CSSPixelsToAppUnits((float)rect->Height())); + nsLayoutUtils::GetFramesForArea(rootFrame, r, frames, + nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | nsLayoutUtils::IGNORE_ROOT_SCROLL_FRAME); + } + if (!frames.Length()) return false; // Detect if we are _inside_ a text control, or something else with its own @@ -1222,89 +1237,7 @@ nsTypeAheadFind::IsRangeVisible(nsIPresShell *aPresShell, (frame->GetStateBits() & NS_FRAME_INDEPENDENT_SELECTION); } - // ---- We have a frame ---- - if (!aMustBeInViewPort) - return true; // Don't need it to be on screen, just in rendering tree - - // Get the next in flow frame that contains the range start - int32_t startRangeOffset, startFrameOffset, endFrameOffset; - aRange->GetStartOffset(&startRangeOffset); - while (true) { - frame->GetOffsets(startFrameOffset, endFrameOffset); - if (startRangeOffset < endFrameOffset) - break; - - nsIFrame *nextContinuationFrame = frame->GetNextContinuation(); - if (nextContinuationFrame) - frame = nextContinuationFrame; - else - break; - } - - // Set up the variables we need, return true if we can't get at them all - const uint16_t kMinPixels = 12; - nscoord minDistance = nsPresContext::CSSPixelsToAppUnits(kMinPixels); - - // Get the bounds of the current frame, relative to the current view. - // We don't use the more accurate AccGetBounds, because that is - // more expensive and the STATE_OFFSCREEN flag that this is used - // for only needs to be a rough indicator - nsRectVisibility rectVisibility = nsRectVisibility_kAboveViewport; - - if (!aGetTopVisibleLeaf && !frame->GetRect().IsEmpty()) { - rectVisibility = - aPresShell->GetRectVisibility(frame, - nsRect(nsPoint(0,0), frame->GetSize()), - minDistance); - - if (rectVisibility != nsRectVisibility_kAboveViewport) { - return true; - } - } - - // We know that the target range isn't usable because it's not in the - // view port. Move range forward to first visible point, - // this speeds us up a lot in long documents - nsCOMPtr frameTraversal; - nsCOMPtr trav(do_CreateInstance(kFrameTraversalCID)); - if (trav) - trav->NewFrameTraversal(getter_AddRefs(frameTraversal), - aPresContext, frame, - eLeaf, - false, // aVisual - false, // aLockInScrollView - false, // aFollowOOFs - false // aSkipPopupChecks - ); - - if (!frameTraversal) - return false; - - while (rectVisibility == nsRectVisibility_kAboveViewport) { - frameTraversal->Next(); - frame = frameTraversal->CurrentItem(); - if (!frame) - return false; - - if (!frame->GetRect().IsEmpty()) { - rectVisibility = - aPresShell->GetRectVisibility(frame, - nsRect(nsPoint(0,0), frame->GetSize()), - minDistance); - } - } - - if (frame) { - nsCOMPtr firstVisibleNode = do_QueryInterface(frame->GetContent()); - - if (firstVisibleNode) { - frame->GetOffsets(startFrameOffset, endFrameOffset); - (*aFirstVisibleRange)->SetStart(firstVisibleNode, startFrameOffset); - (*aFirstVisibleRange)->SetEnd(firstVisibleNode, endFrameOffset); - } - } - - return false; + return true; } already_AddRefed diff --git a/toolkit/components/typeaheadfind/nsTypeAheadFind.h b/toolkit/components/typeaheadfind/nsTypeAheadFind.h index 2caf0b69cc72..9441ea774c58 100644 --- a/toolkit/components/typeaheadfind/nsTypeAheadFind.h +++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.h @@ -56,8 +56,8 @@ protected: // *aNewRange may not be collapsed. If you want to collapse it in a // particular way, you need to do it yourself. bool IsRangeVisible(nsIPresShell *aPresShell, nsPresContext *aPresContext, - nsIDOMRange *aRange, bool aMustBeVisible, - bool aGetTopVisibleLeaf, nsIDOMRange **aNewRange, + nsIDOMRange *aRange, bool aFlushLayout, + nsIDOMRange **aNewRange, bool *aUsesIndependentSelection); nsresult FindItNow(nsIPresShell *aPresShell, bool aIsLinksOnly, bool aIsFirstVisiblePreferred, bool aFindPrev, diff --git a/toolkit/modules/FinderIterator.jsm b/toolkit/modules/FinderIterator.jsm index 121190156801..4c95d095e639 100644 --- a/toolkit/modules/FinderIterator.jsm +++ b/toolkit/modules/FinderIterator.jsm @@ -573,7 +573,8 @@ this.FinderIterator = { let range = window.document.createRange(); range.setStart(frameEl, 0); range.setEnd(frameEl, 0); - if (!finder._fastFind.isRangeVisible(range, this._getDocShell(range), true)) + // Pass `true` to flush layout. + if (!finder._fastFind.isRangeVisible(range, true)) continue; // All conditions pass, so push the current frame and its children on the // stack.