Closed
Bug 1195672
Opened 9 years ago
Closed 9 years ago
Revise AccessibleCaret's long-press logic
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
40 bytes,
text/x-review-board-request
|
roc
:
review+
mtseng
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
roc
:
review+
mtseng
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
roc
:
review+
mtseng
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
roc
:
review+
mtseng
:
review+
|
Details |
The long-press logic of AccessibleCaret has some bugs. One of them is the user cannot reliably long-press a empty content to bring up the text selection dialog as reported in [1].
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1190980#c0
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1195672 - Add |nsAutoCString nsIFrame::ListTag()| for debugging. f=mtseng, r=roc
This make it easier to print a frame tag name in one debug log line.
Attachment #8649728 -
Flags: review?(roc)
Assignee | ||
Comment 2•9 years ago
|
||
Bug 1195672 - Make focus changing by long tap behaves like by single tap. f=mtseng, r=roc
We want the focus changing behavior by long tap as close as to the one
by single tap.
The only functional change is that we always clear old focus and
re-focus the window if a focusable frame cannot be found. This behavior
is the same as the single tap implemented in
EventStateManager::PostHandleEvent().
Besides, ChangeFocus now returns the new focusable frame instead of bool
which provides more information.
Attachment #8649729 -
Flags: review?(roc)
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1195672 - Move the check that frame is selectable into SelectWord. f=mtseng, r=roc
There's a bug that when a frame is focusable but not selectable, we
won't focus on it because we call IsSelectable() before ChangeFocus().
By moving the check into SelectWord(), we'll have a chance to focus on
it.
This resolves a issue that when long press to select a word on a new
opened app, the selection highlight is gray instead of blue.
Attachment #8649730 -
Flags: review?(roc)
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1195672 - Revise the logic of long tap on empty content. f=mtseng, r=roc
The only logic change is that we now call UpdateCaret() before
dispatching CaretStateChangedEvent.
This resolves a bug that the text selection dialog flashes when long
tapping on an empty content.
Attachment #8649731 -
Flags: review?(roc)
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8649728 [details]
MozReview Request: Bug 1195672 - Add |nsAutoCString nsIFrame::ListTag()| for debugging. f=mtseng, r=roc
Bug 1195672 - Add |nsAutoCString nsIFrame::ListTag()| for debugging. f=mtseng, r=roc
This make it easier to print a frame tag name in one debug log line.
Attachment #8649728 -
Flags: review?(mtseng)
Assignee | ||
Updated•9 years ago
|
Attachment #8649729 -
Flags: review?(mtseng)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8649729 [details]
MozReview Request: Bug 1195672 - Make focus changing by long tap behaves like by single tap. f=mtseng, r=roc
Bug 1195672 - Make focus changing by long tap behaves like by single tap. f=mtseng, r=roc
We want the focus changing behavior by long tap as close as to the one
by single tap.
The only functional change is that we always clear old focus and
re-focus the window if a focusable frame cannot be found. This behavior
is the same as the single tap implemented in
EventStateManager::PostHandleEvent().
Besides, ChangeFocus now returns the new focusable frame instead of bool
which provides more information.
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8649730 [details]
MozReview Request: Bug 1195672 - Move the check that frame is selectable into SelectWord. f=mtseng, r=roc
Bug 1195672 - Move the check that frame is selectable into SelectWord. f=mtseng, r=roc
There's a bug that when a frame is focusable but not selectable, we
won't focus on it because we call IsSelectable() before ChangeFocus().
By moving the check into SelectWord(), we'll have a chance to focus on
it.
This resolves a issue that when long press to select a word on a new
opened app, the selection highlight is gray instead of blue.
Attachment #8649730 -
Flags: review?(mtseng)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8649731 [details]
MozReview Request: Bug 1195672 - Revise the logic of long tap on empty content. f=mtseng, r=roc
Bug 1195672 - Revise the logic of long tap on empty content. f=mtseng, r=roc
The only logic change is that we now call UpdateCaret() before
dispatching CaretStateChangedEvent.
This resolves a bug that the text selection dialog flashes when long
tapping on an empty content.
Attachment #8649731 -
Flags: review?(mtseng)
Comment 9•9 years ago
|
||
Comment on attachment 8649728 [details]
MozReview Request: Bug 1195672 - Add |nsAutoCString nsIFrame::ListTag()| for debugging. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16481/#review14765
Ship It!
Attachment #8649728 -
Flags: review?(mtseng) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8649729 [details]
MozReview Request: Bug 1195672 - Make focus changing by long tap behaves like by single tap. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16483/#review14767
Ship It!
Attachment #8649729 -
Flags: review?(mtseng) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8649730 [details]
MozReview Request: Bug 1195672 - Move the check that frame is selectable into SelectWord. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16485/#review14769
Ship It!
Attachment #8649730 -
Flags: review?(mtseng) → review+
Comment 12•9 years ago
|
||
Comment on attachment 8649731 [details]
MozReview Request: Bug 1195672 - Revise the logic of long tap on empty content. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16487/#review14771
Ship It!
Attachment #8649731 -
Flags: review?(mtseng) → review+
Comment on attachment 8649728 [details]
MozReview Request: Bug 1195672 - Add |nsAutoCString nsIFrame::ListTag()| for debugging. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16481/#review14939
Ship It!
Attachment #8649728 -
Flags: review?(roc) → review+
Attachment #8649729 -
Flags: review?(roc) → review+
Comment on attachment 8649729 [details]
MozReview Request: Bug 1195672 - Make focus changing by long tap behaves like by single tap. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16483/#review14941
Ship It!
Comment on attachment 8649730 [details]
MozReview Request: Bug 1195672 - Move the check that frame is selectable into SelectWord. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16485/#review14943
Ship It!
Attachment #8649730 -
Flags: review?(roc) → review+
Comment on attachment 8649731 [details]
MozReview Request: Bug 1195672 - Revise the logic of long tap on empty content. f=mtseng, r=roc
https://reviewboard.mozilla.org/r/16487/#review14945
Ship It!
Attachment #8649731 -
Flags: review?(roc) → review+
Comment 17•9 years ago
|
||
Assignee | ||
Comment 18•9 years ago
|
||
Morris, roc, thank you for the review.
Comment 19•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b356dbdf1043
https://hg.mozilla.org/mozilla-central/rev/233a48d2d378
https://hg.mozilla.org/mozilla-central/rev/6a5121bdb277
https://hg.mozilla.org/mozilla-central/rev/ab8370de21b7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee | ||
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•