User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0 Build ID: 20180704192850 Steps to reproduce: 1. Select an email and press Reply. Composer window opens with the email text (in plain text mode) marked as quotation. 2. Navigate the cursor somewhere in the middle of the quotation. 3. Press Shift+PageDown or Shift+PageUp. Actual results: Nothing happens. Expected results: The cursor should move down or up, selecting the text between the previous and new positions. This works in Thunderbird 52.9.1. I often use these key combinations to quickly select and delete large portions of an email I'm replying to.

I can add that Shift+PageDown/Shift+PageUp do work when the Composer window has a vertical scroll bar. The key combinations stop working if there is no scroll bar.

Masayuki-san, anything in the editor that changed here?

I have no idea. I can reproduce this bug with contenteditable element in Firefox. Needs regression range. https://jsfiddle.net/d_toybox/yptn1zbs/1/

Alice, can you find where this regression occurred, either in TB or FF. Thanks in advance.

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

Thanks Alice. Sorry, I forgot to say "can you *please* ... ", but I'll say Thank You again :-) - Much appreciated.

Really thank you Alice-san! That does make sense. The fix must correct the behavior for the method name. However, the method name is wrong or the caller uses the method with different purpose, that must be PresShell::PageMove(). https://searchfox.org/mozilla-central/rev/4b6737ba4008b71182cc2a12f507c82adf2b4e70/layout/base/PresShell.cpp#2312,2315 That needs a nearest scrollable frame rather than frame *to* scroll. So, perhaps, we need additional API to retrieve it and make PageMove() use it.

PresShell::PageMove() climbs up to parent document when there is no scrollable parent in current document. However, if aExtend is true, it should expand Selection in the document itself. Therefore, it needs different rules to look for container of expanding Selection from scrollable element to scroll. Additionally, old rules (i.e., before the fix of bug 1369072 which caused this regression) were also buggy. It used parent scrollable element or root scrollable element simply. Therefore, if found scrollable element is ancestor of selection limiter, it didn't work as expected. This patch creates nsFrameSelection::GetFrameToPageSelect() to retrieve per-page selection container element with the following rules: - look for a scrollable element in selection limiter. - if there is no scrollable element, use selection limiter. - if there is no selection limiter, use the root frame. So, nsFrameSelection::CommonPageMove() should take nsIFrame rather than nsIScrollableFrame since container of per-page selection may be used in non-scrollable contenteditable element. If it's called with non-scrollable frame, it needs to compute the expanding range with the frame size.

FYI: On macOS, Shift+PageDown/PageUp without editor does not select the content because cmd_selectPageUp/cmd_selectPageDown are not mapped here: https://searchfox.org/mozilla-central/source/dom/xbl/builtin/mac/platformHTMLBindings.xml#33-54 If there is an editor, perhaps, EditorEventListener::KeyPress() resolves native command here: https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/editor/libeditor/EditorEventListener.cpp#636

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/02a07fe87808 PresShell::PageMove() should use different rules to look for a container element for aExtend value r=smaug

Masayuki-san, does that patch apply to m-esr60, then I can take it to TB's release branch. We had various complaints.

[ESR Uplift Approval Request] If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is regression of bug 1369072. User impact if declined: This bug does not allow users to select contents if nearest editing host (e.g., <body> of designMode document or <foo contenteditable>) is not scrollable. Fix Landed on Version: 64 Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): This changes the path to handle per-page selection and the cases are tested by the new automated test. String or UUID changes made by this patch: none

And the patch includes the random orange (bug 1499996), and this may break UX even on Firefox for desktop if composing email is not scrollable. So, this bug may be really annoying for some users who usually use Shift + PageUp/PageDown.

Thanks for the additional comments on severity. Setting the status back to affected for 60.4 consideration. TB may want to graft it to their verbranch for now if they're planning a 60.3.x release with it.

You bet ;-) https://hg.mozilla.org/releases/mozilla-esr60/rev/ab014151d4c338562949c28aa140786b548856ca on THUNDERBIRD_60_VERBRANCH

Works fine in TB 60.3. Thanks for the backport, Masayuki-san.

Unless we see some definite ESR user impact, I'd like to defer this to the next major ESR release. It may be risky and we try to keep change in ESR to major security or stability fixes after the first couple of versions. Good to see that it is now fixed for TB.