Bug 189162 - Make <input type=color> a runtime enabled (on-by-default) feature
Summary: Make <input type=color> a runtime enabled (on-by-default) feature
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-30 11:06 PDT by Aditya Keerthi
Modified: 2018-10-08 18:00 PDT (History)
10 users (show)

See Also:


Attachments
Patch (90.28 KB, patch)
2018-08-30 11:10 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.33 MB, application/zip)
2018-08-30 13:25 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (10.09 MB, application/zip)
2018-08-30 15:23 PDT, EWS Watchlist
no flags Details
Patch (91.37 KB, patch)
2018-08-30 16:41 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Patch (91.36 KB, patch)
2018-08-30 16:46 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Patch (91.67 KB, patch)
2018-08-31 09:36 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Rebase on trunk (93.09 KB, patch)
2018-10-08 13:45 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for landing (93.21 KB, patch)
2018-10-08 14:30 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for landing (1.34 KB, patch)
2018-10-08 16:34 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aditya Keerthi 2018-08-30 11:06:28 PDT
...
Comment 1 Aditya Keerthi 2018-08-30 11:10:43 PDT
Created attachment 348513 [details]
Patch
Comment 2 EWS Watchlist 2018-08-30 13:25:11 PDT
Comment on attachment 348513 [details]
Patch

Attachment 348513 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9037344

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-filter.html
fast/forms/color/input-appearance-color.html
Comment 3 EWS Watchlist 2018-08-30 13:25:13 PDT
Created attachment 348536 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 4 EWS Watchlist 2018-08-30 15:23:39 PDT
Comment on attachment 348513 [details]
Patch

Attachment 348513 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9039230

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/the-form-element/form-elements-filter.html
fast/forms/color/input-appearance-color.html
Comment 5 EWS Watchlist 2018-08-30 15:23:41 PDT
Created attachment 348557 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 6 Aditya Keerthi 2018-08-30 16:41:19 PDT
Created attachment 348566 [details]
Patch
Comment 7 EWS Watchlist 2018-08-30 16:43:16 PDT
Attachment 348566 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/mac/WebColorPickerMac.mm:135:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Aditya Keerthi 2018-08-30 16:46:38 PDT
Created attachment 348567 [details]
Patch
Comment 9 Aditya Keerthi 2018-08-31 09:36:43 PDT
Created attachment 348642 [details]
Patch
Comment 10 Wenson Hsieh 2018-10-01 18:58:15 PDT
Comment on attachment 348642 [details]
Patch

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

> Source/WebKit/UIProcess/mac/WebColorPickerMac.mm:56
> +    __weak id<WKPopoverColorWellDelegate> _webDelegate;

I think we typically use WeakObjCPtr for this.
Comment 11 Wenson Hsieh 2018-10-01 19:01:46 PDT
Comment on attachment 348642 [details]
Patch

We should also make this an experimental feature (by updating WebPreferences.yaml)
Comment 12 Wenson Hsieh 2018-10-08 13:45:06 PDT
Created attachment 351810 [details]
Rebase on trunk
Comment 13 Tim Horton 2018-10-08 13:52:22 PDT
Comment on attachment 351810 [details]
Rebase on trunk

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

> Source/WebKit/Shared/WebPreferences.yaml:1393
> +  humanReadableName: "Input type color"

This is not a good name for the menu

> Source/WebKit/UIProcess/mac/WebColorPickerMac.mm:158
> +    return _webDelegate.get().get();

I think this should be getAutoreleased.
Comment 14 Wenson Hsieh 2018-10-08 13:59:15 PDT
Comment on attachment 351810 [details]
Rebase on trunk

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

>> Source/WebKit/Shared/WebPreferences.yaml:1393
>> +  humanReadableName: "Input type color"
> 
> This is not a good name for the menu

=> "Color Inputs"

>> Source/WebKit/UIProcess/mac/WebColorPickerMac.mm:158
>> +    return _webDelegate.get().get();
> 
> I think this should be getAutoreleased.

👍
Comment 15 Wenson Hsieh 2018-10-08 14:30:51 PDT
Created attachment 351815 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2018-10-08 15:08:49 PDT
Comment on attachment 351815 [details]
Patch for landing

Clearing flags on attachment: 351815

Committed r236942: <https://trac.webkit.org/changeset/236942>
Comment 17 WebKit Commit Bot 2018-10-08 15:08:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Radar WebKit Bug Importer 2018-10-08 15:09:40 PDT
<rdar://problem/45107210>
Comment 19 Wenson Hsieh 2018-10-08 16:34:04 PDT
Reopening to attach new patch.
Comment 20 Wenson Hsieh 2018-10-08 16:34:06 PDT
Created attachment 351833 [details]
Patch for landing
Comment 21 Truitt Savell 2018-10-08 16:39:00 PDT
Looks like this revision: https://trac.webkit.org/changeset/236942/webkit

May have caused an API failure.

Example:
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/builds/13298

Output:
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/13298/steps/run-api-tests/logs/stdio

Excerpt:
Failed

    TestWebKitAPI.DragAndDropTests.DropColor
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/mac/DragAndDropTestsMac.mm:89
        Expected equality of these values:
          @"#ff0000"
            Which is: "#ff0000"
          [webView stringByEvaluatingJavaScript:@"document.querySelector(\"input\").value"]
            Which is: "#ff2600"

Failure seems to be flaky.
Comment 22 Wenson Hsieh 2018-10-08 16:39:58 PDT
(In reply to Truitt Savell from comment #21)
> Looks like this revision: https://trac.webkit.org/changeset/236942/webkit
> 
> May have caused an API failure.
> 
> Example:
> https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/
> builds/13298
> 
> Output:
> https://build.webkit.org/builders/
> Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/13298/steps/run-api-
> tests/logs/stdio
> 
> Excerpt:
> Failed
> 
>     TestWebKitAPI.DragAndDropTests.DropColor
>         
>        
> /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/mac/
> DragAndDropTestsMac.mm:89
>         Expected equality of these values:
>           @"#ff0000"
>             Which is: "#ff0000"
>           [webView
> stringByEvaluatingJavaScript:@"document.querySelector(\"input\").value"]
>             Which is: "#ff2600"
> 
> Failure seems to be flaky.

Ok! Taking a look now...
Comment 23 WebKit Commit Bot 2018-10-08 17:12:01 PDT
Comment on attachment 351833 [details]
Patch for landing

Clearing flags on attachment: 351833

Committed r236951: <https://trac.webkit.org/changeset/236951>
Comment 24 WebKit Commit Bot 2018-10-08 17:12:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 25 Wenson Hsieh 2018-10-08 18:00:25 PDT
(In reply to Wenson Hsieh from comment #22)
> (In reply to Truitt Savell from comment #21)
> > Looks like this revision: https://trac.webkit.org/changeset/236942/webkit
> > 
> > May have caused an API failure.
> > 
> > Example:
> > https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/
> > builds/13298
> > 
> > Output:
> > https://build.webkit.org/builders/
> > Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/13298/steps/run-api-
> > tests/logs/stdio
> > 
> > Excerpt:
> > Failed
> > 
> >     TestWebKitAPI.DragAndDropTests.DropColor
> >         
> >        
> > /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/mac/
> > DragAndDropTestsMac.mm:89
> >         Expected equality of these values:
> >           @"#ff0000"
> >             Which is: "#ff0000"
> >           [webView
> > stringByEvaluatingJavaScript:@"document.querySelector(\"input\").value"]
> >             Which is: "#ff2600"
> > 
> > Failure seems to be flaky.
> 
> Ok! Taking a look now...

Fixing this in <https://bugs.webkit.org/show_bug.cgi?id=190386>.