The general setup is that the State struct is used to iterate over text nodes
explicitly, and keeps references to the ranges so that we don't need to pass all
them around everywhere.
We need to teach nsFindContentIterator to rewind into NAC to be able to get rid
of mIterNode, which was getting out of sync when we failed to rewind to the
anchor node.
MozReview-Commit-ID: 5czYADrm1WX
We're throwing away the computation when aContinueOk is true, so we can remove
that call. Removing that call removes the last usage of aContinueOk, so remove
that handling as well.
This patch is idempotent.
MozReview-Commit-ID: E3sogickWp9
Instead of tweaking member variables and resetting them afterwards, just have an
object that we pass around.
This makes a bit easier to reason about nsFind IMO, and makes us able to use
more complex iterators that don't keep strong references to anything and that
kind of stuff, since we don't keep an iterator member around, and we don't
mutate the DOM from nsFind.
This patch is idempotent.
MozReview-Commit-ID: ERDnL6Q8QTU
We do reset them implicitly next time we call Find(..), since we call
ResetAll() at the beginning of it, then NextNode(..), which unconditionally
overrides them, but this is clearer for the next thing I want to do.
This patch is idempotent.
MozReview-Commit-ID: 6OW8MfkftTM
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
This fixes browser/components/extensions/test/browser/file_find_frames.html with
my patches. We were relying on traversing suppressed whitespace to match the
whole word properly there.
You can see the bug with the following test-case:
<p>Banana 0</p><p>Banana 1</p>
If you try to match "banana" using "Whole word", you'll only find the first
word, because we keep c = '0'. If there's a newline between the two paragraphs,
like in the test, before my patch we we would traverse it (even though it's
suppressed whitespace) and keep c = '\n', which makes the match succeed.
Fix it forgetting the state of the match completely, including c.
That test was firing a lot of "GetOffsetTo() called on frames from different
documents" assertions... That's probably worth looking into as a followup.
MozReview-Commit-ID: AzId7YWQcJI
I ended up not using the nsIFrame methods both for consistency with the plain
text serializer and because of include hell due to nsStyleStructInlines /
nsIFrameInlines.
Find doesn't care about nodes with no frames anyway, so it didn't seem worth
doing the fallback if there's no style information.
I'll file a bug for IsHTMLBlock.
MozReview-Commit-ID: 3T317a4xCB
This fixes browser/components/extensions/test/browser/file_find_frames.html with
my patches. We were relying on traversing suppressed whitespace to match the
whole word properly there.
You can see the bug with the following test-case:
<p>Banana 0</p><p>Banana 1</p>
If you try to match "banana" using "Whole word", you'll only find the first
word, because we keep c = '0'. If there's a newline between the two paragraphs,
like in the test, before my patch we we would traverse it (even though it's
suppressed whitespace) and keep c = '\n', which makes the match succeed.
Fix it forgetting the state of the match completely, including c.
That test was firing a lot of "GetOffsetTo() called on frames from different
documents" assertions... That's probably worth looking into as a followup.
MozReview-Commit-ID: AzId7YWQcJI
I ended up not using the nsIFrame methods both for consistency with the plain
text serializer and because of include hell due to nsStyleStructInlines /
nsIFrameInlines.
Find doesn't care about nodes with no frames anyway, so it didn't seem worth
doing the fallback if there's no style information.
I'll file a bug for IsHTMLBlock.
MozReview-Commit-ID: 3T317a4xCB
This method is not a virtual call, and also looks nicer.
This patch was mostly generated by a Python script, but I manually
cleaned up the code in a few places where statements didn't need to be
split across multiple lines any more.
MozReview-Commit-ID: 8JExxqSRc59
I got a bit carried away with fixing up consumers to use nsINode... But as a
result removing these methods all together made sense.
MozReview-Commit-ID: 2z9Q6D7GY92
(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
This patch adds an overload to nsIContentIterator::Init which accepts
RangeBoundary objects, and modifies the codepath to avoid using the offset of
the start or end nodes to construct the nsIContentIterator.
MozReview-Commit-ID: 5ZqKeiUunoN
nsIContentIterator::Init() takes nsRange but it's too expensive for some users.
So, there should be another Init() which can be specified a range in DOM tree
with 2 pairs of nsINode* and uint32_t.
MozReview-Commit-ID: 6JXic0KOM2d
DOM Standard defines that offset of Range is unsigned long. However, nsRange uses int32_t to them.
This patch makes nsRange use uint32_t instead. However, this patch does NOT allow to set over INT32_MAX as offset values since a lot of users of nsRange cannot treat the values as over INT32_MAX because a lot of internal APIs take int32_t as offsets.
For easier to search such points, this patch adds static_cast<int32_t> to uint32_t variables when they are used for int32_t arguments.
And note that nsContentUtils::ComparePoints() behaves odd. It accepts negative offset and compares such value with valid offset simply. This patch still uses int32_t offset variables in nsRange::CompareNodeToRange() even though it may be negative value if nsINode::IndexOf() returns -1 because the caller of it depends on this behavior.
MozReview-Commit-ID: 8RbOgA86JuT
