295673 - Cannot scroll using keyboard when div with overflow:auto and padding has focus

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Echidnae]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050526 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050526 Camino/0.8+

In the comments section of any mozillazine.org article, selecting text will
cause the arrow keys and keyboard shortcuts like spacebar to lock up.  After
deselecting the text by clicking anywhere inside the comment box, the keys are
still unusable.  Only when you click somewhere outside of the comment box will
the arrows and spacebar work properly again.

Reproducible: Always

Steps to Reproduce:
1. Go to any mozillazine article and scroll down to the comments section.
2. Select some text in a comment and try to use the arrow keys.
3. 

Actual Results:  
Arrow keys, spacebar etc. don't work until you click outside the comment boxes.

Expected Results:  
After selecting text in the comments, the arrow keys and spacebar, etc. continue
to work properly.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Confirming with 20050526.  Works as expected in 0.8.4.  Any idea when this
regressed?

Scrolling on that page also seems a lot slower in the nightlies compared to 0.8.4.

Comment 2

[Mike Pinkerton (not reading bugmail)]

would be nice to have a date on which this regressed. does it break in firefox too?

Comment 3

[Mike Pinkerton (not reading bugmail)]

happens in deerpark

Comment 4

[Mike Pinkerton (not reading bugmail)]

still nice to know what date this started on?

Comment 5

[Echidnae]

Looks like this bug first appeared in the Camino 20050429 nightly.

Comment 6

[Wevah (Nate Weaver)]

Isn't that about the time that the nsGfxScrollFrame changes were checked in?
(Bug 240276)

I'm not sure that this is it, but this came to mind off the top of my head.

Comment 7

[Mike Pinkerton (not reading bugmail)]

changing component

Comment 8

[Mike Pinkerton (not reading bugmail)]

can anyone test if this happens on win/linux as well?

Comment 9

[Jesse Ruderman]

Confirmed on WinXP using the steps in comment 0.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050527
Firefox/1.0+

Comment 10

[Jesse Ruderman]

Attachment: testcase based on a mozillazine article

Comment 11

[Asa Dotzler [:asa]]

roc, can you look at this? it might be fallout from nsGfxScrollframe reorg. 

Comment 12

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

Attachment: fix

This fixes it. GetClientRect returns a rect that's basically "border-box minus
(border and passing)". That's not suitable here; we want the rect that contains
the scrollport and scrollbars, and that's just the border-box minus border.
(Padding goes inside the scrollport.)

Comment 13

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

So GetBorder is an formerly-nsIBox method, and has a comment saying it uses
nsITheme.  Is that really what you want?  Is it consistent with the real border
that's used?

Comment 14

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

It's not the ideal thing, but it's the lowest risk because formerly we were
using GetClientRect which is also an nsIBox method calling GetBorder.

Comment 15

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

Comment on attachment 190364 [details] [diff] [review]
fix

So does it matter that you've changed from parent coordinates to frame
coordinates, or is the frame guaranteed to be placed at (0,0)?

Comment 16

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

(In reply to comment #15)
> So does it matter that you've changed from parent coordinates to frame
> coordinates, or is the frame guaranteed to be placed at (0,0)?

I have? GetClientRect always returned frame coordinates. Maybe I'm not sure what
you're referring to.

Comment 17

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

Sorry, I meant the other way around.  GetClientRect returns coordinates in the
frame itself (since it starts from GetContentRect, which always returns x and y
of 0), whereas GetRect returns parent coordinates.

Comment 18

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

ooh, that's a bug yes. Change to

+nsRect r(nsPoint(0, 0), mOuter->GetSize());

Comment 19

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

Comment on attachment 190364 [details] [diff] [review]
fix

r+sr=dbaron with that change.

Still, I don't see why we get into the state this bug describes even if the
math is wrong.	Is there a good reason for that, or should you file another bug
on that?

Comment 20

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

(In reply to comment #19)
> (From update of attachment 190364 [details] [diff] [review] [edit])
> r+sr=dbaron with that change.

Thanks!

> Still, I don't see why we get into the state this bug describes even if the
> math is wrong.	Is there a good reason for that, or should you file
> another bug on that?

In the testcase, in nsLayoutUtils::GetNearestScrollingView, 'margin' ends up
being -15,-15,-15,-15 and so the test "margin.right" succeeds --- we think we
have a vertical scrollbar and so should scroll vertically if possible.
http://lxr.mozilla.org/mozilla/source/layout/base/nsLayoutUtils.cpp#416

Comment 21

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

Comment on attachment 190364 [details] [diff] [review]
fix

fixes scrolling UI regression

Comment 22

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

checked in

Comment 23

[Echidnae]

Ok, I just checked this on the 20050804 nightlies of both Camino and Deer Park,
and this is still broken.  The testcase works fine on both now, but it's still
not fixed on mozillazine, which was where I first saw this.  Can someone look at
this again?

Comment 24

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

Can you file a new bug with a new testcase?

Comment 25

[Martijn Wargers (dead)]

Attachment: testcase2

Testcase2 based from Mozillazine article that still doesn't scroll with the
keyboard with 2005-08-03 trunk build.

Comment 26

[Martijn Wargers (dead)]

Sorry, didn't see the last comment. I filed bug 303458 for it.