pageup/pagedown does not work properly in contentEditable DIV

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: liucougar, Assigned: liucougar)

Tracking

unspecified
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

11 years ago
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
(Assignee)

Updated

11 years ago
Component: General → Editor
Product: Firefox → Core
QA Contact: general → editor
(Assignee)

Comment 2

10 years ago
Posted patch one possible fix (obsolete) — Splinter Review
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.
Attachment #391969 - Flags: review?(bzbarsky)

Comment 3

10 years ago
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)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

10 years ago
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.
Attachment #391969 - Attachment is obsolete: true
Attachment #391981 - Flags: review?(roc)
Attachment #391969 - Flags: review?(bzbarsky)
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.
Attachment #391981 - Flags: review?(roc) → review+
(Assignee)

Comment 6

10 years ago
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)
Attachment #391981 - Attachment is obsolete: true
Attachment #391998 - Flags: review?(roc)
(Assignee)

Comment 7

10 years ago
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)
(Assignee)

Comment 8

10 years ago
Comment #7 is only applicable to Mac (windows and linux are not affected by Bug 472318)
Depends on: 472318
Keywords: checkin-needed
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/d455294cc585

Thanks!!!
Assignee: nobody → liucougar
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Comment on attachment 391998 [details] [diff] [review]
same patch with mochitest

This already landed.
Attachment #391998 - Flags: review?(roc) → review+

Updated

8 years ago
Depends on: 695660

Updated

7 years ago
Depends on: 775927
You need to log in before you can comment on or make changes to this bug.