Closed Bug 1195672 Opened 9 years ago Closed 9 years ago

Revise AccessibleCaret's long-press logic

Categories

(Core :: DOM: Selection, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

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
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)
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)
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)
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)
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)
Attachment #8649729 - Flags: review?(mtseng)
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.
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)
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 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 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 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 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+
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+
Morris, roc, thank you for the review.
Depends on: 1197739
Blocks: 1197739
No longer depends on: 1197739
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: