[RTL] Text selection handles appear inverted under some circumstances

RESOLVED FIXED in Firefox 54

Status

()

Core
Selection
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: ItielMaN, Assigned: TYLin)

Tracking

(Blocks: 2 bugs, {rtl})

unspecified
mozilla54
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Reporter)

Description

7 months ago
Created attachment 8833261 [details]
Testcase

STR:
1. Install Nightly RTL build
2. Open https://bug849404.bmoattachments.org/attachment.cgi?id=722960
3. Long tap one of the English words- the selectors look okay
4. Long tap one of the Hebrew words- the selectors look inverted

Basically if text that has RTL directionality appears as LTR (and vise versa), the selectors would appear as inverted.
I have attached a testcase that has English and Hebrew text that appear on different directionality declarations for you to test.

Comment 1

7 months ago
Hi TY,
Per talked, would you help to take a look on this bug please? Thank you.
Flags: needinfo?(tlin)
(Assignee)

Comment 2

7 months ago
This bug only occurs on Fennec because Fennec enables "layout.accessiblecaret.always_tilt". Interesting, Google Chrome (on Android) seems to have the same issue once the user starts dragging caret. on the text that has RTL directionality appears as LTR (and vise versa).

I can reproduce this issue on Firefox Desktop with the following pref.
* "layout.accessiblecaret.enabled" sets to true.
* "layout.accessiblecaret.hide_carets_for_mouse_input" sets to true.
* "layout.accessiblecaret.always_tilt" sets to true.

This bug occurs only on Fennec because Fennec enables "layout.accessiblecaret.always_tilt".
Blocks: 1124074
Flags: needinfo?(tlin)
(Assignee)

Comment 3

7 months ago
The caret tilt direction is controlled by [1]. Currently, we only look at the directionality of the frame, not the directionality of the text. I admit the tilted carets look awkward if the directionality of the text is in a content container with opposite "dir" attribute such as English words are in the middle of Hebrew text [2]. The tilted carets might be overlapping under such circumstances. 

[1] http://searchfox.org/mozilla-central/rev/848c29538ab007fb95dc6cff194f0e3d3809613d/layout/base/AccessibleCaretManager.cpp#453-466
[2] Hebrew Wikipedia's Firefox page https://he.wikipedia.org/wiki/%D7%9E%D7%95%D7%96%D7%99%D7%9C%D7%94_%D7%A4%D7%99%D7%99%D7%A8%D7%A4%D7%95%D7%A7%D7%A1
(Assignee)

Comment 4

7 months ago
Created attachment 8834789 [details]
carets-overlapped.png

I don't feel the direction of the carets on LTR text in RTL environment dampens the usability. The real issue is the overlapping carets like the screenshot shown prevents the user from adjusting the selection range.

I see two options to fix this bug.

1) Use the language's directionality to tilt the caret. However, this approach doesn't work well. The caret will swing from left to right (or vice versa) when being dragging across the boundary of RTL and LTR text or a space (because the "space" character doesn't have directionality)

2) The direction of the two carets stays the same like it was, but we separate the inward carets when they're too close. We use the same approach like "layout.accessiblecaret.always_tilt" is off. I'll post a patch later.
(Assignee)

Comment 5

7 months ago
Created attachment 8834793 [details] [diff] [review]
Use character's directionality to tilt caret. (Approach 1 in comment 4)

Attach the patch for approach 1 in comment 4 for the record. It doesn't work well.
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Assignee: nobody → tlin
Status: NEW → ASSIGNED
(Assignee)

Updated

7 months ago
Component: Text Selection → Selection
Product: Firefox for Android → Core

Comment 7

7 months ago
mozreview-review
Comment on attachment 8834796 [details]
Bug 1336388 - Use overlapping tilt mechanism in always tilt mode.

https://reviewboard.mozilla.org/r/110628/#review112008
Attachment #8834796 - Flags: review?(mtseng) → review+

Comment 8

7 months ago
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7feeeaab8221
Use overlapping tilt mechanism in always tilt mode. r=mtseng

Comment 9

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7feeeaab8221
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Woot! Thank you Ting-yu for the quick turnaround!
(Reporter)

Comment 11

6 months ago
This patch indeed fixes the carets being overlapped but it doesn't cover all scenarios, including what I've mentioned in comment 0.
Can it be addressed in this bug or should I open a new one?
Flags: needinfo?(tlin)
(Assignee)

Comment 12

6 months ago
I'm aware that the carets would appear as inverted on text that has RTL directionality in dir=ltr environment (and vise versa). However, using text's directionality to make them look normal doesn't seem good to me (see the reason of approach 1 in comment 4). A third approach could be that we use the absolute x-position of the carets to decide their appearance, but the carets will still swing back and forth when their x-position exchanged.

To keep the caret's appearance being consistent tilted to either left or right in either LTR or RTL environment regardless of the text's directionality, the inverted carets is a natural consequence.
Flags: needinfo?(tlin)
You need to log in before you can comment on or make changes to this bug.