Closed Bug 1111517 Opened 10 years ago Closed 10 years ago

selection in vertical-rl content with nested blocks is flaky or broken

Categories

(Core :: Layout: Block and Inline, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file testcase for selection behavior (deleted) —
STR: in vertical-enabled build, load attached testcase. Click within the first (rightmost) of the text, and drag gradually leftwards. Note how the selection is correct when the cursor is within one of the lines, but whenever it is in the inter-line gaps, everything to the end of the text gets selected. This produces an ugly, distracting flicker if the cursor is dragged back and forth. For a more extreme example, load attachment 8525944 [details] (testcase from bug 1102175) and set writing mode to vertical-rl; then try to drag-select some of the text. Drag-select works within the labels of the radio buttons, but does not work at all within the main <div>. In other writing modes, however, it is fine.
This occurs because of an error in the patch from bug 1088025. That used nsBlockFrame::SlideLine to move the lines of a vertical-rl block into their final positions once the block size (which becomes their container width) is known. This correctly updates the rects of the lines and their child frames, so everything displays in the proper place; however, it is wrong because it also updates the *logical* rect of the nsLineBox records. At this point, where the lines have been reflowed logically, but rects were set based on an incorrect container width, what we need to do is to maintain their logical rects *unchanged*, but update physical positions based on the new container width. So although the effect on physical rects and hence visual appearance is the same as SlideLine, this is a different operation in terms of logical coordinates.
With this, drag-selection behaves much better.
Attachment #8536446 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8536446 [details] [diff] [review] Use new method UpdateLineContainerWidth instead of SlideLine when finalizing the width of a vertical-rl block Review of attachment 8536446 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsBlockFrame.cpp @@ +2813,5 @@ > + return; > + } > + > + // Adjust line state > + nscoord widthDelta = aLine->UpdateContainerWidth(aNewContainerWidth); Move this inside the if().
Comment on attachment 8536446 [details] [diff] [review] Use new method UpdateLineContainerWidth instead of SlideLine when finalizing the width of a vertical-rl block Review of attachment 8536446 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsBlockFrame.cpp @@ +2813,5 @@ > + return; > + } > + > + // Adjust line state > + nscoord widthDelta = aLine->UpdateContainerWidth(aNewContainerWidth); Forget that; we should update it for consistency regardless of the writing mode.
Comment on attachment 8536446 [details] [diff] [review] Use new method UpdateLineContainerWidth instead of SlideLine when finalizing the width of a vertical-rl block Review of attachment 8536446 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8536446 - Flags: review?(smontagu) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: