Closed
Bug 1128187
Opened 10 years ago
Closed 10 years ago
Allow SelectionHandles in mixed LTR-RTL content
Categories
(Firefox for Android Graveyard :: Text Selection, defect)
Tracking
(firefox38 fixed)
RESOLVED
FIXED
Firefox 38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: capella, Assigned: capella)
Details
Attachments
(2 files)
11.28 KB,
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
1.29 KB,
patch
|
Details | Diff | Splinter Review |
While testing the correction for bug 1081742, I realized that SelectionHandler will have to be made smarter following it's completion.
Currently, the selection handles take their RTL/LTR cue from the document, but pages (in and out of RM) can display a mix of both. Handles can start and end in elements with different text directions ... testcase [1]
[1] https://www.dropbox.com/s/ik1ks4mhhe0js6k/test_bug1081742.html?dl=0
Assignee | ||
Comment 1•10 years ago
|
||
The fix isn't too hard, and while exposed by bug 1081742, exists w/o it based on design of the underlying webpage.
Mixed content selection w/o fix:
https://www.dropbox.com/s/ysrd0278wmjj0x0/bug1128187_Incorrect.png?dl=0
Mixed content selection w/the fix:
https://www.dropbox.com/s/0nmd5lg0lg9odhy/bug1128187_Correct.png?dl=0
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #8557517 -
Flags: review?(margaret.leibovic)
Comment 2•10 years ago
|
||
Comment on attachment 8557517 [details] [diff] [review]
bug1128187.diff
Review of attachment 8557517 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, nice job with the code comments! Sorry for the delayed review, I'm getting back on top of things now!
::: mobile/android/chrome/content/SelectionHandler.js
@@ +1202,5 @@
> + * Return true if text associated with a node is RTL.
> + */
> + _isNodeRTL: function(node) {
> + // Find containing node that supports .direction attribute (needed
> + // when target node is #text for example).
Nice comment.
Attachment #8557517 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Drats ... dropped a piece for the "TextSelection:PositionHandles" generated for Caret Handle ... not used, but needed to avoid "org.json.JSONException: No value for rtl" in TextSelection.java
Assignee | ||
Comment 6•10 years ago
|
||
verbal by rnewman
https://tbpl.mozilla.org/?tree=Fx-Team&rev=946346db74bb
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0a92f400101e
https://hg.mozilla.org/mozilla-central/rev/946346db74bb
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•