390423 - Simplify the signature of nsIFrame::GetPointFromOffset

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Eli Friedman]

Currently, nsIFrame::GetPointFromOffset takes both a prescontext and a rendering context; it needs neither one.

Comment 1

[Ben Karel [eschew]]

Attachment: Initial attempt at fix

This fixes all the usages of GetPointFromOffset that lxr knows of, and updates the IID of nsIFrame.

I also ended up modifying the method signatures of PresShell::CreateRangePaintInfo and PresShell::ClipListToRange, as they took nsIRenderingContext pointers solely to pass to GetPointFromOffset.

Comment 2

[Eli Friedman]

Comment on attachment 274964 [details] [diff] [review]
Initial attempt at fix

Looks good to me!

You'll want to request sr from roc, and an additional review from Aaron Leventhal ([e-mail]) for the accessibility changes.

Comment 3

[Aaron Leventhal]

Comment on attachment 274964 [details] [diff] [review]
Initial attempt at fix

r+ just for mozilla/accessible changes.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 274964 [details] [diff] [review]
Initial attempt at fix

nice work! need something to do next? :-)

Comment 5

[Ben Karel [eschew]]

Attachment: Update to tip

This fixes a CVS conflict that occurred in accessible/src/html/nsHyperTextAccessible.cpp after I created the original patch. I also took the liberty of replacing "OFfset" with "Offset" in that file, as I assumed that "OFfset" is a search-and-replaced misspelling. All the other parts of the diff are unchanged (apart from being more up-to-date, of course).

As I don't have commit privs, if someone could check in, that'd be dandy.

Comment 6

[Aaron Leventhal]

We also have a patch that fixes that misspelling which is awaiting approval.

Comment 7

[Aaron Leventhal]

Comment on attachment 276109 [details] [diff] [review]
Update to tip

You need approval right now before it can get checked in, since we're in semi-lock-down. (See details on Firefox tbox page).

We also have a patch that fixes that misspelling in mozilla/accessible which is awaiting approval. It might get checked in before yours does -- you don't really need to fix the misspelling but it's no problem if you do.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 276109 [details] [diff] [review]
Update to tip

>-  PRUint32 startRenderedOFfset, endRenderedOFfset;
>+  PRUint32 startRenderedOffset, endRenderedOFfset;

I find it curious that you're fixing the spelling of one variable but not the other.

a1.9=dbaron

Comment 9

[Aaron Leventhal]

As I said, those spelling errors were already fixed by me in another bug. I just checked in, so you'll have to merge.

Comment 10

[Ben Karel [eschew]]

Attachment: Second update

Patch merged against the latest nsHyperTextAccessible.cpp. All other files same as before.

Comment 11

[Phil Ringnalda (:philor)]

layout/base/nsCaret.cpp 1.179
layout/base/nsPresShell.cpp 3.1046
accessible/src/html/nsHyperTextAccessible.cpp 1.73
accessible/src/msaa/nsTextAccessibleWrap.cpp 1.25
accessible/src/msaa/nsTextAccessibleWrap.h 1.8
layout/generic/nsFrame.cpp 3.747
layout/generic/nsFrame.h 3.270
layout/generic/nsIFrame.h 3.376
layout/generic/nsSelection.cpp 3.291
layout/generic/nsTextFrameThebes.cpp 3.69

Comment 12

[Ben Karel [eschew]]

Attachment: fix compiler warning

I forgot to remove a now-unused presContext variable, leading to a compiler warning. Oops!

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 277201 [details] [diff] [review]
fix compiler warning

rs+a+alphabet-soup=me