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)
Core
Layout: Block and Inline
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: jfkthame, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 2•10 years ago
|
||
With this, drag-selection behaves much better.
Attachment #8536446 -
Flags: review?(smontagu)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
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().
Assignee | ||
Comment 4•10 years ago
|
||
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 5•10 years ago
|
||
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+
Assignee | ||
Comment 6•10 years ago
|
||
Target Milestone: --- → mozilla37
Comment 7•10 years ago
|
||
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.
Description
•