Closed Bug 1195672 Opened 4 years ago Closed 4 years ago

Revise AccessibleCaret's long-press logic

Categories

(Core :: DOM: Selection, defect)

defect
Not set

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.