284924 - Visible scrollbar won't scroll vertically with keyboard

Status: NEW

(Core :: XUL, enhancement)

Description

[Tristan Miller]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041220
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041220

The web page <http://www.seasip.info/VintagePC/cga.html> displays normally, but
I cannot scroll using any of the following:

1. Up arrow
2. Down arrow
3. Home key
4. End key
5. Mouse scroll wheel

The following does work, however:

1. Clicking on the scroll bar
2. Find text as you type
3. Ctrl+F
4. Ctrl+G after #2 or #3

I'm not sure what's causing this.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Tristan Miller]

After further testing, it seems the window can be scrolled, but only when there
is an active link.  For example, one must first middle-click on a hyperlink to
open in it in a new tab.  The page can now be scrolled as long as the link is
active.  If one clicks anywhere else on the page to deactivate the link, the
page returns to its non-scrollable state.

Comment 2

[Mats Palmgren (inactive)]

Mouse-wheel scrolling has been fixed in newer builds (bug 97283).

Keyboard scrolling does not work because there is no overflow of the root
element to scroll to (the page consists of two fixed pos. DIVs).
(focus the one with the scrollbar and keyboard scrolling works fine).

As an enhancement, maybe we could look for a fixed pos. frame that is
scrollable for the keyboard case? (when nothing is focused)

Comment 3

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 4

[Mats Palmgren (inactive)]

Having a look at this...

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 7

[Aaron Leventhal]

Comment on attachment 176424 [details] [diff] [review]
Patch rev. 1

+  return ((aDirection == nsLayoutUtils::eHorizontal ||
+	    aDirection == nsLayoutUtils::eEither) &&
+	   portRect.width < scrolledSize.width) ||
+	  ((aDirection == nsLayoutUtils::eVertical ||
+	    aDirection == nsLayoutUtils::eEither) && 
+	   portRect.height < scrolledSize.height);

How about a slight simplification like:
+  return (aDirection != nsLayoutUtils::eVertical &&
+	   portRect.width < scrolledSize.width) ||
+	  (aDirection != nsLayoutUtils::eHorizontal && 
+	   portRect.height < scrolledSize.height);

Also, what if there is nothing with position: fixed, but there is something
with overflow:scroll or overflow:auto
Should we be able to scroll those children if the main doc has no scrollbars?

Anyway, r=aaronlev
Nice improvment.

Comment 8

[Mats Palmgren (inactive)]

(In reply to comment #7)

> Also, what if there is nothing with position: fixed, but there is something
> with overflow:scroll or overflow:auto

I guess the intention here is to catch "frame like" designs where one of the
positioned elements is used instead of the viewport to scroll the main
portion of the content (like the URL).
I don't think we should look at normal flow.
Maybe we should look in the root 'absoluteList' also though...

Comment 9

[Boris Zbarsky [:bzbarsky]]

> I guess the intention here is to catch "frame like" designs 

Those can use a non-positioned div with overflow set for the main content,
though.  For example, see http://web.mit.edu/bzbarsky/www/index.shtml (and
change overflow on the ContentHolder to "auto" while setting a fixed height for it).

I'm really not sure what the right behavior here is...  Note that for a "framed"
design we would need a click in (or tab to) the frame to scroll it.  That
already works, doesn't it?

Comment 10

[Zook Valem]

Can confirmed this bug on my machine and patch is already ready to go...

Status --> New

Comment 11

[Zook Valem]

>>patch is already ready to go...
Mistyped the word "already" for "almost"...  Sorry about that...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Again, I'm still not quite sure why this patch is what we want...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176424 [details] [diff] [review]
Patch rev. 1

I'm not convinced this is right.

Comment 14

[Tristan Miller]

This bug seems to be fixed.  Scrolling works fine with the original page I reported, as long as with the test case.

I'm using Mozilla/5.0 (X11; U; Linux x86_64; en-GB; rv:1.9.1.16) Gecko/20101124 SUSE/2.0.11-2.2 SeaMonkey/2.0.11.

Is it OK to close this as FIXED?

Comment 15

[Mats Palmgren (inactive)]

(In reply to comment #14)
> This bug seems to be fixed.  Scrolling works fine with the original page I
> reported,

I suspect that the URL has changed since the "main" div is now position:relative
and that's why keyboard scrolling now works there.

> as long as with the test case.

The testcase still shows the reported problem.

Fwiw, I still think my suggested fix makes sense:  when there is no element
focused and the viewport is unscrollable, then look for a position:fixed
element that is scrollable and scroll that.

It would make keyboard scrolling work for "frame-like presentations" using
position:fixed envisioned in CSS2:
http://www.w3.org/TR/CSS2/visuren.html#fixed-positioning

Comment 16

[Leif Halvard Silli]

Bug 682763 relates to this bugs: if the "div frame" has @tabindex set, then it becomes impossible to use the SPACE key to scroll the "div frame". Even clicking inside the "div frame" doesn't work then.

@Mats: 

I may not understand your fix correct, but after fiddling with bug 6827763, I have the following questions/issues regarding your fix:

* HTML5: http://dev.w3.org/html5/spec/editing#focus
* HTML5 specifies that it is only when an element has focus that it should answer to key events. It also says that no element might be focused and that, in that case, the "root" element must be the target for keyevents.
* Note as well that HTML5 says that elements that have focus answer to *:focus{}. So, I don't think your fix is correct unless it also causes the element to answer to *:focus{}

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.