438840 - pageup/pagedown does not work properly in contentEditable DIV

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[liucougar]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008061105 Minefield/3.0pre

Load the attached test case, focus into the contentEditable DIV (such as clicking on the first paragraph), pageup/pagedown does not work: it moves the caret to the element before the contentEditable DIV when pageup is pressed, or to the element after the contentEditable DIV if pagedown is pressed

Reproducible: Always

Steps to Reproduce:
1. load the attach test case in latest FF3 nightly
2. clicking somewhere in the contentEditable DIV (such as the first paragraph)
3. press pageUp/pageDown
Actual Results:  
when pageUp is pressed, the caret is moved to the element before the contentEditable DIV (in the test page, it is the "first row" DIV)

when pageDown is pressed, the caret is moved to the element after the contentEditable DIV

Expected Results:  
pageUp/pageDown should only move the caret within the contentEditable DIV

Comment 1

[liucougar]

Attachment: testcase for pagedown/up bug in contentEditable element

Comment 2

[liucougar]

Attachment: one possible fix

The problem is caused by the fact that when pageup/pagedown is pressed in a contentEditable area, it actually scrolls the entire page up/down, rather than the contentEditable area itself.

I have no idea how this should be fixed. By trial and error, I found this possible fix: whenever a editor is focused, if it can scroll, set it as the root scollable view, so when pageup/pagedown is pressed in the editor, it will scroll the editor rather than the entire page.

I think this is not the properly fix, but I just want to show it to get more feedback/pointers on where to look.

Comment 3

[cmtalbert]

Confirmed on Windows Minefield per comment 0's steps.  I also noticed that page up took me out of the content editable box onto the "first row" area.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090731 Minefield/3.6a1pre (.NET CLR 3.5.30729)

Comment 4

[liucougar]

Attachment: proposed patch: use the same logic as in non-editable page scroll

Thanks for roc's suggestion, I believe this patch is now on the right direction

in the original PresShell::PageMove, it always scrolls the page using the root scrollable view. by scrolling GetViewToScroll instead, this bug is fixed.

Comment 5

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

Comment on attachment 391981 [details] [diff] [review]
proposed patch: use the same logic as in non-editable page scroll

I like this!

Would be nice to have an automated test for this. Perhaps something that uses synthesizeKey to synthesize a page-down.

Comment 6

[liucougar]

Attachment: same patch with mochitest

same patch as the previous one, with added mochitest case

(I don't have a Mac, so I don't know whether it works as expected in Mac, but if so, I think this patch also fixes Bug 472318)

Comment 7

[liucougar]

I believe this patch is required to make the richtext edtior case work in Bug 472318 (the patch in that bug is required as well)

Comment 8

[liucougar]

Comment #7 is only applicable to Mac (windows and linux are not affected by Bug 472318)

Comment 9

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

http://hg.mozilla.org/mozilla-central/rev/d455294cc585

Thanks!!!

Comment 10

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

Comment on attachment 391998 [details] [diff] [review]
same patch with mochitest

This already landed.