Bug 141210 – [iOS] Selection Callout should not immediately disappear on pages with frequent layouts

• Component: WebKit2
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
Selection Callout should not immediately disappear on pages with frequent layouts. At the same time, the caret should be blinking, instead of staying constant blue.

* STEPS TO REPRODUCE
1. Load http://archive.org/web/
2. Focus text field
3. Attempt to paste
  => callout dismisses immediately after showing

* NOTES
- JavaScript animation is triggering layout -> triggers selection update -> dismisses callout and re-updates caret

rdar://problem/18301801

[Joseph Pecoraro]

Created attachment 245966 [details]
[PATCH] Proposed Fix

Not really sure if there is a way to test this. Any pointers?

[WebKit Commit Bot]

Attachment 245966 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:78:  The parameter name "editorState" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Created attachment 245972 [details]
[PATCH] Proposed Fix

Fixed style.

[Enrica Casucci]

Comment on attachment 245966 [details]
[PATCH] Proposed Fix

The patch looks good.I'm assuming you tested zooming on a page where a field is focused. Please check the windows build failure before submitting.

[Enrica Casucci]

Comment on attachment 245966 [details]
[PATCH] Proposed Fix

The patch looks good.I'm assuming you tested zooming on a page where a field is focused. Please check the windows build failure before submitting.

[Joseph Pecoraro]

(In reply to comment #6)
> Comment on attachment 245966 [details]
> [PATCH] Proposed Fix
> 
> The patch looks good.I'm assuming you tested zooming on a page where a field
> is focused.

Yep, testing zooming is exactly why there is the force path.


> Please check the windows build failure before submitting.

This patch doesn't modify anything for Windows, so I'll assume it was just a passing thing.

[WebKit Commit Bot]

Comment on attachment 245972 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 245972

Committed r179578: <http://trac.webkit.org/changeset/179578>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sam Weinig]

Comment on attachment 245972 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=245972&action=review

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:75
> +struct WKSelectionDrawingInfo {

This does not need, and should not have, a WK prefix. I realize there is a counter example right below, but that is wrong.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:76
> +    enum class SelectionType { None, Plugin, Range };

We usually put this on multiple lines.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:81
> +    WKSelectionDrawingInfo();
> +    explicit WKSelectionDrawingInfo(const EditorState&);
> +    SelectionType type;
> +    WebCore::IntRect caretRect;
> +    Vector<WebCore::SelectionRect> selectionRects;

This would read nicer if you put a space between the constructors and the members.