C++ createTreeWalker callers broken by API change

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
4 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla21
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

The API change in bug 698384 didn't update any callers to the new API.  That doesn't matter too much for JS callers, since the extra argument is ignored.  But for C++ callers what used to be the "entityReferenceExpansion" boolean is now treated as the optional_argc.

There are two C++ callers.  One was passing true, which becomes 1, and it was passing a null filter so it's ok.  The other one, though, was passing false, so now its whatToShow argument is being ignored.
Can we add a test that would have caught this?
I don't know.  The only real difference in behavior was for the accessibility treewalker...

Trevor, any idea how to test that?
Comment on attachment 710591 [details] [diff] [review]
part 1.  Fix treewalker callers per the API change that was made.

Ugh.
Attachment #710591 - Flags: review?(bugs) → review+
Attachment #710592 - Flags: review?(bugs) → review+
> Trevor, any idea how to test that?

Well, testing this will suck, and I should probably rip the nsIDOM crud there out so any test you write will hopefully stop testing anything soon.  However if you really want to test it I think something like <div>foo bar</div> selecting "bar" then getting the accessible for that div and asking its nsIAccessibleText impl for the selection bounds should work.
I doubt that will work.  The bug here was that the treewalker was using SHOW_ALL instead of SHOW_ELEMENT|SHOW_TEXT.  So to trigger it we'd need a tree that has other things in there (comments, processing instructions, something)...
(In reply to Boris Zbarsky (:bz) from comment #7)
> I doubt that will work.  The bug here was that the treewalker was using
> SHOW_ALL instead of SHOW_ELEMENT|SHOW_TEXT.  So to trigger it we'd need a

yeah, oops, thanks xpidl

> tree that has other things in there (comments, processing instructions,
> something)...

In that case I'm not sure it has an observable effect since that loop ignores things that don't have accessibles, and all we use it for is text offset stuff.
Great, thanks!
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.