1932800 - Caret on many text fields (E.g. address-bar, b.m.o search field, etc.) appears to be shifted upwards

Status: RESOLVED FIXED

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

Description

[Mayank Bansal]

STR1
Create new profile
Click on the address-bar

AR: The mouse-cursor appears to be shifted upwards
ER: Not so

STR2
Click on the search-bar at the top of b.m.o
AR: cursor appears shifted upwards

Bisection:
Bug 1931466: Ensure inline elements inside an empty line has zero block-size and no baseline in scrolled block frames. r=layout-reviewers,dholbert
Differential Revision: https://phabricator.services.mozilla.com/D229709

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1931466

Comment 3

[Alice0775 White]

I can confirm the issue.
The caret(text insertion point) is shifted upwards if the input field is empty.
STR:
1 open data:text/html,<input autofocus>

Comment 5

[Mark Banner (:standard8)]

[Tracking Requested - why for this release]: Obvious UI visual regressions to the caret in text fields.

Comment 6

[Pascal Chevrel:pascalc]

Tracking for 134

Comment 8

[Emilio Cobos Álvarez [:emilio]]

This was already kind of an issue before, apparently: data:text/html,<div contenteditable autofocus><span>

Comment 10

[Pascal Chevrel:pascalc]

Setting it as a 134 blocker as it impacts most websites with forms and the regression is visible.

Comment 11

[Alice0775 White]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

This was already kind of an issue before, apparently: data:text/html,<div contenteditable autofocus><span>

Sorry off-topic but in comment#8 case,

Regression window in comment#8 is as follows:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2f45c30741c5&tochange=422bbd8245a7
Suspect: db8e95fbcd39b56b7ae7ad0877e774f9e046949b 	Ehsan Akhgari — Bug 644428 - Position the caret correctly for empty inline frames; r=roc

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

(This also affects the address bar)

Comment 14

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1932800 - Fix caret baseline of empty inline and text frames. r=dshin

Added Gecko reftests because I don't know what the best place to put
them in WPT is, and caret rendering is kind of undefined, but...

Comment 15

[Sean Feng [:sefeng211]]

(Added two more crash bugs as blocks because they look related)

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1932800 - Use ShouldHaveLineIfEmpty() to guarantee buttons have at least one line of height. r=dshin

This is similar to how we treat <div contenteditable>, and allows us to
remove the specialness about anon boxes.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/jobs?repo=try&revision=8eec59976905c5ffee14e71cd687aca811343fab

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1932800 - Fold IsEmpty check into SelfIsSelectable(). r=dshin,masayuki,#layout

All the callers check both conditions, so no behavior change.

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1932800 - Add a check to SelfIsSelectable to preserve editor drag and drop behavior. r=masayuki

We were relying on editable text frames not being empty. We fixed that
on the previous patches but this caused editor d&d to break.

For now this preserves the selection behavior. In the future we should
look into removing the IsEmpty() condition or so, probably, or make it
more subtle.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f6758d53d7f
Fold IsEmpty check into SelfIsSelectable(). r=dshin

Comment 21

[Emilio Cobos Álvarez [:emilio]]

Updated try runs:

• https://treeherder.mozilla.org/jobs?repo=try&revision=3cbdf72317c576342892bb72a288422bab7072ca has all reftests passing, doesn't have comment 19 which is what fixes the remaining failures.
• https://treeherder.mozilla.org/jobs?repo=try&revision=2c6d2a4d0907b48c5c3ca827b8ab66e1ed1e6bea is still in progress but should be fully green.

Comment 22

[Emilio Cobos Álvarez [:emilio]]

The IsEmpty special-cases begin in bug 316281. It's not clear to me why they are needed.

Comment 23

[Emilio Cobos Álvarez [:emilio]]

That's only mentioned in bug 316281 comment 23. Will try to remove in a separate bug.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee6527afae3b
Add a check to SelfIsSelectable to preserve editor drag and drop behavior. r=masayuki

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/333442d03d1a
Fix caret baseline of empty inline and text frames. r=dshin
https://hg.mozilla.org/integration/autoland/rev/048956318eb0
Use ShouldHaveLineIfEmpty() to guarantee buttons have at least one line of height. r=dshin
https://hg.mozilla.org/integration/autoland/rev/c06b939ca2f2
apply code formatting via Lando

Comment 26

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6f6758d53d7f

Comment 27

[Timothy Nikkel (:tnikkel)]

You pushed the last patch, should leave-open be removed now?

Comment 28

[Sandor Molnar[:smolnar]]

Backed out for causing reftest failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/23dc330a38878592e7c716f962f610e00d90392a

Push with failures

Failure log -> REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/css-ui/caret-empty-contenteditable-02.html == layout/reftests/css-ui/caret-empty-contenteditable-ref.html

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4974ccf274fa
Fix caret baseline of empty inline and text frames. r=dshin
https://hg.mozilla.org/integration/autoland/rev/7cda61151052
Use ShouldHaveLineIfEmpty() to guarantee buttons have at least one line of height. r=dshin
https://hg.mozilla.org/integration/autoland/rev/f40a44add850
apply code formatting via Lando

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a47e016449ef
Fix reftest-snapshot annotations, and one AccessibleCaret test.

Comment 31

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/4974ccf274fa
https://hg.mozilla.org/mozilla-central/rev/7cda61151052
https://hg.mozilla.org/mozilla-central/rev/f40a44add850
https://hg.mozilla.org/mozilla-central/rev/a47e016449ef

Comment 32

[agoloman]

Hi! Your push causes marionette perma failures. Could you take a look?

Comment 33

[agoloman]

Backed out for causing marionette perma failures @test_accessiblecaret_cursor_mode.py.

• Backout link 1
• Backout link 2
• Push with failures
• Failure Log

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0a2ef9d7ae3
Fix caret baseline of empty inline and text frames. r=dshin
https://hg.mozilla.org/integration/autoland/rev/93a86915295b
Use ShouldHaveLineIfEmpty() to guarantee buttons have at least one line of height. r=dshin

Comment 35

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/b0a2ef9d7ae3
https://hg.mozilla.org/mozilla-central/rev/93a86915295b

Comment 36

[Julien Cristau [:jcristau]]

Add a check to SelfIsSelectable to preserve editor drag and drop behavior. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/ee6527afae3b