Closed Bug 1583135 Opened 5 years ago Closed 4 years ago

Entered text scrolled out of view after Ctrl+Home on long document followed by entering text and hitting enter

Categories

(Core :: Layout: Scrolling and Overflow, defect, P3)

Product:
Core
Component:
Layout: Scrolling and Overflow
Version:
68 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla74
Tracking Flags:
Tracking Status
thunderbird_esr60 --- unaffected
thunderbird_esr68 --- affected
firefox-esr60 --- unaffected
firefox-esr68 --- verified
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- verified
firefox74 --- verified

People

(Reporter: craig, Assigned: masayuki)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

long-email.html
14.20 KB, text/html
Details
Bug 1583135 - Disable scroll-anchoring at all editable elements r=emilio!
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr68+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36

Steps to reproduce:

Open email
Click reply
Click control-home to type at top of email
Enter a line of text
Press return
The text just typed disappears off the top of the window and you have to scroll up to see it.
Until recently, the window auto scrolled and kept the top line in view.
This is annoying, as I'd like to see what I've typed.

Using 68.1.0 (32-bit)

Actual results:

Text unexpectedly scrolls out of sight

Expected results:

Text is still visible.

Confirmed.

If you really want to answer above the quote, I suggest you configure that in the account settings: Composition & A dressing: "Start my reply above the quote".

The downside of your Ctrl+Home method is that you insert your text into the "cite prefix":
<div class="moz-cite-prefix">On 28 Aug 2019 13:17, someone wrote:</div>

That has some undesired side effects.

That said, we can still look where the scrolling behaviour changed, but it's most likely an issue in the Mozilla platform code.

Alice, can you please find the regression for us:
Configure TB to start the reply below the quote. Reply to a long e-mail, hit Ctrl+Home, type something into the cite prefix, hit enter. What you just types has now scrolled out of view.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(alice0775)

Configure TB to start the reply below the quote
How?

the cite prefix
What is?

Flags: needinfo?(alice0775) → needinfo?(jorgk)

In the account settings under Composition & A dressing: "Start my reply below the quote".

When you reply, you'll automatically get a "On 28 Aug 2019 13:17, someone wrote:". That's the cite prefix. So if you do Ctrl+Home and start typing, you'll type into the cite prefix before the "On".

Flags: needinfo?(jorgk)
Attached file long-email.html —

Thank you so much, Alice.

As I suspected, this comes from a change in the Mozilla platform code. It actually doesn't happen in Firefox, if you open the attached page and type right a the front and hit return, the typed line is not scrolled out of view.

I don't think will devote too much effort into fixing this, Craig, please try the preference or follow the steps in comment #2.

I can reproduce the issue with the Attached file long-email.html on Nightly71.0a1 Windows10.

Steps

  1. Open Attached file long-email.html
  2. Scroll to bottom
  3. Click somewhere so that caret will appear
  4. hit Ctrl+Home,
  5. type something
  6. hit [Enter] key

I can also reproduce the issue on Firefox68.0.2.

Thanks again, Alice. I was wrong. The key is to scroll to the bottom first, then click there, then use Ctrl+Home, then type, then hit enter. I'm still on FF 60.9 ESR, so I don't see it there, but of course it's present in the current released version.

Summary: Compose reply window not auto scrolling → Entered text scrolled out of view after Ctrl+Home on long document followed by entering text and hitting enter

Looks like somewhat related to editor.

Priority: -- → P3

Too late for 70 but we could still take a patch in 71.

status-firefox70: affected → wontfix

Masayuki-san, could you look at this?

Hmm, if it caused by scroll anchoring, it's really not my area and also it'd be removed entirely.
https://groups.google.com/forum/#!topic/mozilla.dev.platform/66jW_XKCZew

I can probably take a look if it's a scroll anchoring regression, but probably you just need to set overflow-anchor: none on the relevant node to get the previous behavior.

Hmm, that would be somewhere in the <editor> element in the TB compose window. I'll take a look.

Flags: needinfo?(jorgk)

STR

  1. Start Nightly72.0a1
  2. Open https://www-archive.mozilla.org/editor/midasdemo/
  3. type something and hit Enter
  4. Repeat step3 until the editor will be overflowed
  5. Crtl+Home
  6. type something and hit Enter

Actual results:
Last typed text in step6 unexpectedly scrolls out of sight
Chrome does not.
So this is definitely Firefox core bug.

Thanks for the comment, Alice. So in view of comment #17 there shouldn't be a Thunderbird-only solution, right?

Flags: needinfo?(jorgk)

Well, I mean, for the point of view of scroll-anchoring, this is working as expected: Pressing enter inserts content on top of the anchor, so in order to keep the anchor visually on top we adjust the scroll position back. The issue is that in this case the content insertion moving stuff down is what the user wants, because the user is editing, so the page or the editor should use overflow-anchor: none.

Maybe Gecko should have overflow-anchor: none in contenteditable regions by default somehow, so that editable content isn't chosen as an anchor, but setting overflow-anchor: none on the editable html document in comment 17 fixes it for me.

Masayuki, what do you think about comment 19?

Flags: needinfo?(masayuki)

Yeah, sounds reasonable to me. But this might not be reproducible only with contenteditable but also <textarea> and <input> (when its font-size is too big). And for avoiding other regressions, the style should be applied only when :forcus pseudo-class is matched. But unfortunately, in designMode, nobody can be focused. So, the style could be:

*:-moz-read-write:focus,
*:root:-moz-read-write {
  overflow-anchor: none;
}

?

Flags: needinfo?(masayuki) → needinfo?(emilio)

I'm not sure about textarea and input, but the scroller in those is the anonymous div I think, so we could apply overflow-anchor: none to that.

The rule in comment 21 sounds reasonable to me, without the redundant * selectors.

Flags: needinfo?(emilio)

We have shipped our last beta for 71 and this is an unfixed P3, so this is wontfix for 71.

status-firefox71: affected → wontfix
status-firefox72: --- → affected

Masayuki, are you planning to return to this for 73?

status-firefox72: affected → fix-optional
status-firefox73: --- → affected
Flags: needinfo?(masayuki)

Well, I try to write a patch for the CSS and create test described in comment 17.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)

Sigh, the symptom is not reproducible with synthesized events... So, a patch comes without automated test.

Marking this as fix-optional for 73 to remove it from weekly regression triage. We can still take a patch for 73 or 74.

status-firefox72: fix-optional → wontfix
status-firefox73: affected → fix-optional

This patch disables scroll-anchoring of all editable elements including editable
and scrollable elements in editing host because users don't want to scroll down
when they insert new line and new line can be in the scrolled area.

Unfortunately, the bug is not reproducible with simple testcase with
synthesized keyboard events. Therefore, this patch does not include automated
tests.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3cbc74c83a6e
Disable scroll-anchoring at all editable elements r=emilio

https://hg.mozilla.org/mozilla-central/rev/3cbc74c83a6e

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox74: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

This looks pretty low-risk to uplift if you want to nominate it for Beta and ESR68 approval.

Flags: needinfo?(masayuki)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #33)

This looks pretty low-risk to uplift if you want to nominate it for Beta and ESR68 approval.

WDYT, emilio?

Flags: needinfo?(masayuki) → needinfo?(emilio)

Seems fair to me, I agree it's pretty low risk.

Flags: needinfo?(emilio)

Comment on attachment 9121215 [details]
Bug 1583135 - Disable scroll-anchoring at all editable elements r=emilio!

Beta/Release Uplift Approval Request

  • User impact if declined: Typing something in rich text editor causes unexpected scroll.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 17.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just disabling scroll-anchoring in editable content.
  • String changes made/needed:
Attachment #9121215 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified as fixed on Windows 10 x64 and on Windows 7 x86 on Firefox Nightly 74.0a1 (2020-01-21).

status-firefox74: fixed → verified

Comment on attachment 9121215 [details]
Bug 1583135 - Disable scroll-anchoring at all editable elements r=emilio!

Disables scroll anchoring in editable elements to avoid unexpected scroll behavior in some situations. Approved for 73.0b8 and 68.5esr.

Attachment #9121215 - Flags: approval-mozilla-esr68+
Attachment #9121215 - Flags: approval-mozilla-beta?
Attachment #9121215 - Flags: approval-mozilla-beta+

Thanks, TB users will appreciate this (and we don't have to do it ourselves).

Verified as fixed on Windows 10 x64 and on Windows 7 x86 on Firefox 73.0b8.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
status-firefox73: fixed → verified
Flags: qe-verify+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified fixed with Firefox 68.5.0esr (20200203194112) on Windows 10x64 and Windows 7x64.

status-firefox-esr68: fixed → verified
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: