227313 - unused code related to PresShell::ScrollFrameIntoView(nsIFrame*)

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

The one parameter version of ScrollFrameIntoView on PresShell / nsIFocusTracker
is unused, as is the pres shell's mScrollingEnabled member variable and the
EnableScrolling and DisableScrolling methods.  See:

http://lxr.mozilla.org/seamonkey/search?string=EnableScrolling
http://lxr.mozilla.org/seamonkey/search?string=DisableScrolling
http://lxr.mozilla.org/seamonkey/search?string=mScrollingEnabled
http://lxr.mozilla.org/seamonkey/search?string=ScrollFrameIntoView

Comment 1

[Ludovic Hirlimann [:Usul]]

Attachment: Patch removing unused code

Comment 2

[Ludovic Hirlimann [:Usul]]

Should I ask review to someone in particular ?

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... can you explain why your change to, eg,
STDMETHODIMP nsAccessNodeWrap::scrollTo(/* [in] */ boolean aScrollTopLeft)
is correct? or really any of the changes that removes a caller of
ScrollFrameIntoView?

-  NS_IMETHOD ScrollFrameIntoView(nsIFrame *aFrame,
-                                 PRIntn   aVPercent, 
-                                 PRIntn   aHPercent) const = 0;

this doesn't look like "The one parameter version of ScrollFrameIntoView on
PresShell / nsIFocusTracker"...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Ludovic, reasonable reviewers for this change would be dbaron, roc, myself,
bryner, or rbs.  You should indeed ask someone in particular for review.

Note that I'm not going to be able to do any reviews till mid-July, though.

Comment 5

[Ludovic Hirlimann [:Usul]]

Comment on attachment 150132 [details] [diff] [review]
Patch removing unused code

I was too focused on the lxr results. Only the one pamareter version of
ScrollFrameIntoView is to be removed.

Comment 6

[Ludovic Hirlimann [:Usul]]

Attachment: patch adressing the bug

Comment 7

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

Comment on attachment 150601 [details] [diff] [review]
patch adressing the bug

cool

Comment 8

[Ludovic Hirlimann [:Usul]]

Anyone to check this one in ?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in layout/base/public/nsIFocusTracker.h;
/cvsroot/mozilla/layout/base/public/nsIFocusTracker.h,v  <--  nsIFocusTracker.h
new revision: 1.16; previous revision: 1.15
done
Checking in layout/html/base/src/nsPresShell.cpp;
/cvsroot/mozilla/layout/html/base/src/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.737; previous revision: 3.736
done