1712834 - Caret incorrectly appears at the previous line end even when the newline character is significant

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Attachment: caret-newline-1.html

Bug 612271 introduced a hack for text controls to workaround this, but the general problem still exists and the old workaround is now causing bug 1712248.

I took a quick look and found nsTextFrame::GetPointFromIterator returns a point for the line end, not the next line start. Not sure how to fix this, I can only think of another hack for this (getting a point of the next BRFrame in a certain condition).

Emilio, what do you think?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I think generally we shouldn't even be getting at GetPointForIterator. IIRC we have the concept of "adjusted" and "unadjusted" frame to deal with this kind of case. Is AdjustCaretFrameForLineEnd not doing the right thing? I would expect that to move the caret frame appropriately. If that's not doing the right thing, that'd probably need fixing, rather than hacking around it in nsTextFrame.

Comment 2

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

AdjustCaretFrameForLineEnd does nothing, because the frame is already the trailing text frame. Per the comment it seems its intention is the reverse of mine?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Ah, I see. Well, but this is not an issue about <br> or what not right? It's an issue about preserved whitespace entirely. In particular, if you replace <br> with \n in your test-case, I see the same issue, and I get this frame tree:

              Block(pre)(0)@7f1ce28b3cc0 parent=7f1ce28b3ba8 (x=0, y=0, w=1008, h=29) [content=7f1ce2903e50] [cs=7f1ce59b1d38] <
                line@7f1ce28b3e28 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,before:nobr,after:linebr[0x80100] (x=0, y=0, w=21.65, h=14.5) <
                  Text(0)"ABC\n"@7f1ce28b3d88 parent=7f1ce28b3cc0 next=7f1ce28b3ec8 next-in-flow=7f1ce28b3ec8 (x=0, y=0, w=21.65, h=14.5) [content=7f1ce2904380] [cs=7f1ce59a7c58:-moz-text] [run=7f1ce58c9dc0][0,4,F] 
                >
                line@7f1ce28b3f70 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,before:nobr,after:linebr[0x80100] (x=0, y=14.5, w=0, h=14.5) <
                  Text(0)"\n"@7f1ce28b3ec8 parent=7f1ce28b3cc0 prev-in-flow=7f1ce28b3d88 (x=0, y=14.5, w=0, h=14.5) [content=7f1ce2904380] [cs=7f1ce59a7c58:-moz-text] [run=7f1ce58c9dc0][4,1,T] 
                >

In any case GetPointForIterator is definitely not the right place to handle this. Either GetFrameForNodeOffset (this bit of code looks relevant, except it doesn't handle text nodes) or the AdjustFrame* seem like the right place to handle this.

Comment 4

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

Attachment: Bug 1712834 - Adjust caret frame for a significant terminal newline r=emilio

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/edf78628c7cf
Adjust caret frame for a significant terminal newline r=emilio

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29154 for changes under testing/web-platform/tests

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/edf78628c7cf

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot