PgUp/PgDown doesn't move cursor in mail compose

RESOLVED FIXED in Firefox 66

Status

()

defect
RESOLVED FIXED
9 months ago
4 months ago

People

(Reporter: thomas.hebinck, Assigned: masayuki)

Tracking

({regression})

60 Branch
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(thunderbird_esr60? affected, firefox-esr60 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Details

()

Attachments

(2 attachments)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0

Steps to reproduce:

Open mail compose (plain text), have text less than the height of the window, press PageUp/PageDown keys.


Actual results:

Nothing.


Expected results:

The cursor should move to the top/bottom of the text (as before).
Exact version is: 60.2.1 (64-bit) on Ubuntu
Status: UNCONFIRMED → RESOLVED
Closed: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1482425

Oops, this is not a duplicate of bug 1482425 since that one is about Shift+PageUp/Down.

I've just tried, yes, in TB 52 on a document with no scrollbar, the cursor moves when PageUp/Down are used.

Alice, can you please find the regression.

Status: RESOLVED → REOPENED
Ever confirmed: true
Flags: needinfo?(alice0775)
Resolution: DUPLICATE → ---
Status: REOPENED → NEW
Duplicate of this bug: 1517928
Duplicate of this bug: 1518067
Duplicate of this bug: 1517928

Regression window(comm-central):
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=def874d2d27c8fa1734f885059d29362b3990032&tochange=4fc52379f051d1bfc91eda6cf789d2d2119e3526
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=50857982881ae7803ceb438fee90650a282f7f05&tochange=c71b01e993510268bab7d60154b2f80692fd507d

And the problem can be reproduced in Firefox57 and Nightly66.0a1 as well as in Thunderbird.

STR:

  1. data:text/html,<div contentEditable="true" style="height:100px; border:1px solid black;"><p>foo bar baz</p><p>hoge huga piyo</p></div>
  2. Put caret on 1st line
  3. press PageUp/PageDown keys

Regression window(mozilla-central):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=50857982881ae7803ceb438fee90650a282f7f05&tochange=dd3736e98e4e7558af2f202803bce36278a26c66

Suspect:
ef1641e40903 Masayuki Nakano — Bug 1369072 - part3: nsXBLPrototypeHandler::DispatchXBLCommand() should use controller of visible window r=smaug
1c3ac6cb53ea Masayuki Nakano — Bug 1369072 - part2: PresShell::HandleEvent() should retarget KeyboardEvent if focused document is invisible r=smaug
19f8a5d3da39 Masayuki Nakano — Bug 1369072 - part1: PresShell should climb up scrollable frames when there is no selection, no focused content and root scrollable frame isn't scrollable r=smaug
2e507cfef60d Masayuki Nakano — Bug 1369072 - part0: Add automated test for testing key event handlers to scroll parent document when an iframe element has or had focus r=smaug

Flags: needinfo?(alice0775)
Flags: needinfo?(masayuki)

reporter of bug 1518067 "Note that this does work once the body text gets longer than the window. Once the scrollbar appears, page up and page down start working normally."

Keywords: regression
OS: Unspecified → All

Thank you very much, Alice. So bug 1369072 regressed both bug 1482425 (Shift+PageUp/Down) as well as this bug here (PageUp/Down).

Blocks: 1369072

Sigh... Why we don't have this kind of tests...

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
Hardware: Unspecified → All
If there is no scrollable frame, PresShell::GetScrollableFrameToScroll() returns
nullptr.  However, even when we don't expand selection, we need to move caret
in current selection root.  Therefore, it should call
nsFrameSelection::CommonPageMove() with the result of
nsFrameSelection::GetFrameToPageSelect() to move caret.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/87e21b003b07
Make PresShell::PageMove() use result of nsFrameSelection::GetFrameToPageSelect() when PresShell::GetScrollableFrameToScroll() returns nullptr r=smaug

Can I have an m-esr60 version for this, please.

(In reply to Jorg K (GMT+1) from comment #14)

Can I have an m-esr60 version for this, please.

Cannot it be applied directly? It's really small patch?

Flags: needinfo?(jorgk)

Not quite, there seems to be an issue in the test, well, I tried applying to our THUNDERBIRD_60_VERBRANCH where I have already backported bug 1482425.

$ hg qpush
applying 87e21b003b07
patching file layout/base/tests/mochitest.ini
Hunk #1 FAILED at 145
1 out of 1 hunks FAILED -- saving rejects to file layout/base/tests/mochitest.ini.rej
patching file layout/base/tests/test_moving_and_expanding_selection_per_page.html
Hunk #1 FAILED at 17
1 out of 2 hunks FAILED -- saving rejects to file layout/base/tests/test_moving_and_expanding_selection_per_page.html.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 87e21b003b07

But the reject file is small.

BTW, this isn't strictly a small patch, is it? 664 lines, 37 KB.

Flags: needinfo?(jorgk)

Wow, really odd. The test change is big, but the test was new for bug 1482425. So, only the different points should be mochitest.ini and PresShel.cpp, but they are enough small.

Have not you applied the patch for bug 1499996? It fixed the random orange of the new test for bug 1482425, though. (You usually see random oranges of the test?)

I've never heard of bug 1499996. Maybe it would be best if you did an ESR60 patch for me, so I don't mess it up ;-)

You can do: hg up -C THUNDERBIRD_60_VERBRANCH

Component: Message Compose Window → Keyboard: Navigation
Product: Thunderbird → Core
Version: 60 → 60 Branch

Ah, okay, the patch for bug 1499996 has already been merged. The difference is how to get docShell.

I'll attach a patch after build and running the test, but I'm not sure if I can attach it before going to bed.

Thanks, absolutely no rush :-)

Thanks, I'll get it landed for TB 60.5 ESR!

Flags: needinfo?(jorgk)
Status: ASSIGNED → RESOLVED
Closed: 9 months ago7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.