Bug 192172 – Make drawing tools available when an editable image is focused

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Make drawing tools available when an editable image is focused

[Tim Horton]

Created attachment 356036 [details]
Patch

[Tim Horton]

<rdar://problem/30337960>

[EWS Watchlist]

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


ERROR: Source/WebKit/Platform/spi/ios/PencilKitSPI.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 21 files


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

[EWS Watchlist]

Comment on attachment 356036 [details]
Patch

Attachment 356036 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10201642

New failing tests:
accessibility/svg-image.html
accessibility/mac/aria-liveregion-on-image.html
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
accessibility/presentational-elements-with-focus.html
fast/events/tabindex-focus-blur-all.html

[EWS Watchlist]

Created attachment 356049 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Tim Horton]

Created attachment 356050 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/Platform/spi/ios/PencilKitSPI.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 21 files


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

[EWS Watchlist]

Comment on attachment 356050 [details]
Patch

Attachment 356050 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10203206

New failing tests:
accessibility/presentational-elements-with-focus.html
accessibility/mac/aria-liveregion-on-image.html
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
accessibility/svg-image.html
fast/events/tabindex-focus-blur-all.html

[EWS Watchlist]

Created attachment 356063 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Tim Horton]

Created attachment 356064 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/Platform/spi/ios/PencilKitSPI.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 22 files


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

[Dean Jackson]

Comment on attachment 356064 [details]
Patch

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

> Source/WebCore/editing/InsertEditableImageCommand.cpp:55
> +    m_imageElement->setAttributeWithoutSynchronization(styleAttr, AtomicString("display: block", AtomicString::ConstructFromLiteral));

Should this be block? Image attachments in Mail are inline.

> Source/WebKit/UIProcess/ios/WKInkPickerControl.mm:83
> +    CGSize keyboardSize = [UIKeyboard defaultSizeForInterfaceOrientation:[UIApp interfaceOrientation]];
> +    return [_inlinePicker sizeThatFits:keyboardSize];

Does this try to make the ink picker the same size as what a keyboard would be? I thought it was less tall?

> Source/WebKit/UIProcess/ios/WKInkPickerControl.mm:94
> +    CGSize keyboardSize = [UIKeyboard defaultSizeForInterfaceOrientation:[UIApp interfaceOrientation]];

You probably should use the size parameter here rather than ask for it again :)

[Tim Horton]

Comment on attachment 356064 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/WKInkPickerControl.mm:83
>> +    return [_inlinePicker sizeThatFits:keyboardSize];
> 
> Does this try to make the ink picker the same size as what a keyboard would be? I thought it was less tall?

sizeThatFits returns the smaller height. The keyboard provides the /maximum/ size.

>> Source/WebKit/UIProcess/ios/WKInkPickerControl.mm:94
>> +    CGSize keyboardSize = [UIKeyboard defaultSizeForInterfaceOrientation:[UIApp interfaceOrientation]];
> 
> You probably should use the size parameter here rather than ask for it again :)

I don't know what you mean! We're disregarding the input size and just saying that our sizeThatFits is always precisely the width of the keyboard and the height that the ink picker wants.

[Dean Jackson]

Comment on attachment 356064 [details]
Patch

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

>>> Source/WebKit/UIProcess/ios/WKInkPickerControl.mm:94
>>> +    CGSize keyboardSize = [UIKeyboard defaultSizeForInterfaceOrientation:[UIApp interfaceOrientation]];
>> 
>> You probably should use the size parameter here rather than ask for it again :)
> 
> I don't know what you mean! We're disregarding the input size and just saying that our sizeThatFits is always precisely the width of the keyboard and the height that the ink picker wants.

My mistake. I thought this was the sizeThatFits that you were calling from inkPickerSize above.

[Tim Horton]

https://trac.webkit.org/changeset/238708/webkit

[Tim Horton]

Reopening to attach new patch.

[Tim Horton]

Created attachment 356100 [details]
Patch

[Tim Horton]

Nope, webkit-patch did the wrong thing.