Closed Bug 295673 Opened 17 years ago Closed 17 years ago

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

Categories

(Core :: DOM: Selection, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: echidnae, Assigned: roc)

References

()

Details

(Keywords: access, regression, verified1.8)

Attachments

(3 files)

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.
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.
Keywords: regression
Status: UNCONFIRMED → NEW
Ever confirmed: true
would be nice to have a date on which this regressed. does it break in firefox too?
Target Milestone: --- → Camino0.9
happens in deerpark
Assignee: pinkerton → mozeditor
Component: General → Editor
Product: Camino → Core
QA Contact: bugzilla
Target Milestone: Camino0.9 → ---
Version: unspecified → Trunk
still nice to know what date this started on?
Severity: normal → critical
Flags: blocking-aviary1.1?
Looks like this bug first appeared in the Camino 20050429 nightly.
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.
changing component
Assignee: mozeditor → selection
Component: Editor → Selection
QA Contact: bugzilla
can anyone test if this happens on win/linux as well?
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+
OS: MacOS X → All
Hardware: Macintosh → All
Summary: Arrow keys become locked when text is selected (at mozillazine) → Cannot scroll using keyboard when div with overflow:auto and padding has focus
Keywords: access
Flags: blocking-aviary1.1? → blocking1.8b4?
roc, can you look at this? it might be fallout from nsGfxScrollframe reorg. 
Assignee: selection → roc
Attached patch fixSplinter Review
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.)
Attachment #190364 - Flags: superreview?(dbaron)
Attachment #190364 - Flags: review?(dbaron)
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?
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 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)?
(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.
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.
ooh, that's a bug yes. Change to

+nsRect r(nsPoint(0, 0), mOuter->GetSize());
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?
Attachment #190364 - Flags: superreview?(dbaron)
Attachment #190364 - Flags: superreview+
Attachment #190364 - Flags: review?(dbaron)
Attachment #190364 - Flags: review+
(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 on attachment 190364 [details] [diff] [review]
fix

fixes scrolling UI regression
Attachment #190364 - Flags: approval1.8b4?
Attachment #190364 - Flags: approval1.8b4? → approval1.8b4+
checked in
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
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?
Can you file a new bug with a new testcase?
Attached file testcase2
Testcase2 based from Mozillazine article that still doesn't scroll with the
keyboard with 2005-08-03 trunk build.
Sorry, didn't see the last comment. I filed bug 303458 for it.
Flags: blocking1.8b4?
Keywords: fixed1.8
Keywords: fixed1.8verified1.8
You need to log in before you can comment on or make changes to this bug.