Bug 198615 – Remove Dashboard support

• Component: New Bugs
• Status: RESOLVED FIXED

[Sam Weinig]

Remove Dashboard support

[Sam Weinig]

Created attachment 371514 [details]
Patch

[Sam Weinig]

Created attachment 371516 [details]
Patch

[Tim Horton]

Comment on attachment 371516 [details]
Patch

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

> Source/WebCore/css/CSSPrimitiveValue.h:114
> -        CSS_UNICODE_RANGE = 102,
> +        CSS_UNICODE_RANGE = 101,

Odd, are these all explicit numbers for a reason?

> Source/WebCore/css/StyleResolver.cpp:-121
> -#if ENABLE(DASHBOARD_SUPPORT)
> -#endif
> -
> -#if ENABLE(VIDEO_TRACK)
> -#endif

WTF

> Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:47
>  GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) U_DISABLE_RENAMING=1 U_SHOW_CPLUSPLUS_API=0 $(GCC_PREPROCESSOR_DEFINITIONS_$(PLATFORM_NAME));

Do we need this anymore?

> Tools/ImageDiff/cg/Configurations/Base.xcconfig:38
>  GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) $(GCC_PREPROCESSOR_DEFINITIONS_$(PLATFORM_NAME));

Do we need this anymore?

[Sam Weinig]

(In reply to Tim Horton from comment #3)
> Comment on attachment 371516 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371516&action=review
> 
> > Source/WebCore/css/CSSPrimitiveValue.h:114
> > -        CSS_UNICODE_RANGE = 102,
> > +        CSS_UNICODE_RANGE = 101,
> 
> Odd, are these all explicit numbers for a reason?

I was wondering that too. They also aren't IDL constants or anything, so they can probably not shout at us. I'll modernize it as a follow up.

> 
> > Source/WebCore/css/StyleResolver.cpp:-121
> > -#if ENABLE(DASHBOARD_SUPPORT)
> > -#endif
> > -
> > -#if ENABLE(VIDEO_TRACK)
> > -#endif
> 
> WTF
> 
> > Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:47
> >  GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) U_DISABLE_RENAMING=1 U_SHOW_CPLUSPLUS_API=0 $(GCC_PREPROCESSOR_DEFINITIONS_$(PLATFORM_NAME));
> 
> Do we need this anymore?

Nope, will fix.

> 
> > Tools/ImageDiff/cg/Configurations/Base.xcconfig:38
> >  GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) $(GCC_PREPROCESSOR_DEFINITIONS_$(PLATFORM_NAME));
> 
> Do we need this anymore?

Nope, will fix.

[Sam Weinig]

*** Bug 198624 has been marked as a duplicate of this bug. ***

[Sam Weinig]

Created attachment 371535 [details]
Patch

[Sam Weinig]

Anyone know what's going on with the inability to apply the change?

Failed to find the property value for the SVN property "svn:mergeinfo": "Deleted: svn:mime-type
". at /home/ews-gtk/WebKit/Tools/Scripts/VCSUtils.pm line 1546, <ARGV> line 4056.

[Sam Weinig]

Created attachment 371539 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/css/CSSPrimitiveValue.h:114:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 48 files


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

[Sam Weinig]

Created attachment 371541 [details]
Patch

[Sam Weinig]

(In reply to Sam Weinig from comment #7)
> Anyone know what's going on with the inability to apply the change?
> 
> Failed to find the property value for the SVN property "svn:mergeinfo":
> "Deleted: svn:mime-type
> ". at /home/ews-gtk/WebKit/Tools/Scripts/VCSUtils.pm line 1546, <ARGV> line
> 4056.

I just manually removed the svn:mergeinfo lines from the patch and it seems to work. Fun.

[EWS Watchlist]

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


ERROR: Source/WebCore/css/CSSPrimitiveValue.h:114:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 48 files


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

[EWS Watchlist]

Comment on attachment 371541 [details]
Patch

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

New failing tests:
fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html
fast/scrolling/latching/scroll-iframe-fragment.html
fast/scrolling/latching/scroll-iframe-in-overflow.html
fast/frames/flattening/scrolling-in-object.html
fast/scrolling/latching/scroll-iframe-webkit1-latching-bug.html
fast/scrolling/latching/iframe_in_iframe.html
fast/scrolling/rtl-point-in-iframe.html
fast/scrolling/latching/scroll-div-no-latching.html
compositing/fixed-with-main-thread-scrolling.html
fast/scrolling/latching/scroll-latched-nested-div.html
fast/scrolling/latching/scroll-nested-iframe.html
fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html
fast/events/scroll-in-scaled-page-with-overflow-hidden.html

[EWS Watchlist]

Created attachment 371548 [details]
Archive of layout-test-results from ews101 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 371541 [details]
Patch

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

New failing tests:
http/wpt/service-workers/service-worker-networkprocess-crash.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 371541 [details]
Patch

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

New failing tests:
fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html
fast/scrolling/latching/scroll-iframe-fragment.html
fast/scrolling/latching/scroll-iframe-in-overflow.html
fast/frames/flattening/scrolling-in-object.html
fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html
fast/scrolling/latching/iframe_in_iframe.html
fast/scrolling/rtl-point-in-iframe.html
fast/scrolling/latching/scroll-div-no-latching.html
compositing/fixed-with-main-thread-scrolling.html
fast/events/scroll-in-scaled-page-with-overflow-hidden.html
fast/scrolling/latching/scroll-nested-iframe.html
fast/scrolling/latching/scroll-iframe-webkit1-latching-bug.html
fast/scrolling/latching/scroll-latched-nested-div.html

[EWS Watchlist]

Created attachment 371551 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 371541 [details]
Patch

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

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases
apiTests

[Sam Weinig]

Created attachment 371707 [details]
Patch

[EWS Watchlist]

Comment on attachment 371707 [details]
Patch

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

New failing tests:
fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html
fast/scrolling/latching/scroll-iframe-fragment.html
fast/scrolling/latching/scroll-iframe-in-overflow.html
fast/frames/flattening/scrolling-in-object.html
fast/scrolling/latching/scroll-iframe-webkit1-latching-bug.html
fast/scrolling/latching/iframe_in_iframe.html
fast/scrolling/rtl-point-in-iframe.html
fast/scrolling/latching/scroll-div-no-latching.html
compositing/fixed-with-main-thread-scrolling.html
fast/scrolling/latching/scroll-latched-nested-div.html
fast/scrolling/latching/scroll-nested-iframe.html
fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html
fast/events/scroll-in-scaled-page-with-overflow-hidden.html

[EWS Watchlist]

Created attachment 371708 [details]
Archive of layout-test-results from ews101 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 371707 [details]
Patch

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

New failing tests:
fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html
fast/scrolling/latching/scroll-iframe-fragment.html
fast/scrolling/latching/scroll-iframe-in-overflow.html
fast/frames/flattening/scrolling-in-object.html
fast/scrolling/latching/scroll-iframe-webkit1-latching-bug.html
fast/scrolling/latching/iframe_in_iframe.html
fast/scrolling/rtl-point-in-iframe.html
fast/scrolling/latching/scroll-div-no-latching.html
compositing/fixed-with-main-thread-scrolling.html
fast/scrolling/latching/scroll-latched-nested-div.html
fast/scrolling/latching/scroll-nested-iframe.html
fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html
fast/events/scroll-in-scaled-page-with-overflow-hidden.html

[EWS Watchlist]

Created attachment 371711 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Sam Weinig]

Created attachment 371714 [details]
Patch

[Ryosuke Niwa]

Windows failure:
C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/TestNetscapePlugIn.exp
c:\cygwin\home\buildbot\webkit\tools\dumprendertree\win\testrunnerwin.cpp(695): error C2039: 'setUseDashboardCompatibilityMode': is not a member of 'TestRunner'

[Darin Adler]

Comment on attachment 371714 [details]
Patch

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

Exciting patch. Those "FIXME: Remove" comments are calling my name!

> Source/WebKitLegacy/mac/WebView/WebClipView.mm:-187
> -- (void)scrollWheel:(NSEvent *)event
> -{
> -    NSView *docView = [self documentView];
> -    if ([docView respondsToSelector:@selector(_webView)]) {
> -#if ENABLE(DASHBOARD_SUPPORT)
> -        WebView *wv = [docView _webView];
> -        if ([wv _dashboardBehavior:WebDashboardBehaviorAllowWheelScrolling]) {
> -            [super scrollWheel:event];
> -        }
> -#endif
> -        return;
> -    }
> -    [super scrollWheel:event];
> -}

This is a behavior change. Before, this would just return and not call super if the documentView responds to the selector _webView.

Is the behavior change an accident, or intentional?

[Sam Weinig]

(In reply to Darin Adler from comment #27)
> Comment on attachment 371714 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371714&action=review
> 
> Exciting patch. Those "FIXME: Remove" comments are calling my name!
> 
> > Source/WebKitLegacy/mac/WebView/WebClipView.mm:-187
> > -- (void)scrollWheel:(NSEvent *)event
> > -{
> > -    NSView *docView = [self documentView];
> > -    if ([docView respondsToSelector:@selector(_webView)]) {
> > -#if ENABLE(DASHBOARD_SUPPORT)
> > -        WebView *wv = [docView _webView];
> > -        if ([wv _dashboardBehavior:WebDashboardBehaviorAllowWheelScrolling]) {
> > -            [super scrollWheel:event];
> > -        }
> > -#endif
> > -        return;
> > -    }
> > -    [super scrollWheel:event];
> > -}
> 
> This is a behavior change. Before, this would just return and not call super
> if the documentView responds to the selector _webView.
> 
> Is the behavior change an accident, or intentional?

I actually don't think this is a behavior change since this method is only ever called in mac Legacy WebKit, where ENABLE(DASHBOARD_SUPPORT) was true, and [wv _dashboardBehavior:WebDashboardBehaviorAllowWheelScrolling] always returned true by default (this is due to the underlying value, dashboardBehaviorAllowWheelScrolling in WebViewData.mm being initialized to YES, unlike it's peers, that default initialize to NO).

[Sam Weinig]

Created attachment 371776 [details]
For landing

[WebKit Commit Bot]

Comment on attachment 371776 [details]
For landing

Clearing flags on attachment: 371776

Committed r246285: <https://trac.webkit.org/changeset/246285>

[Darin Adler]

Comment on attachment 371714 [details]
Patch

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

>>> Source/WebKitLegacy/mac/WebView/WebClipView.mm:-187
>>> -}
>> 
>> This is a behavior change. Before, this would just return and not call super if the documentView responds to the selector _webView.
>> 
>> Is the behavior change an accident, or intentional?
> 
> I actually don't think this is a behavior change since this method is only ever called in mac Legacy WebKit, where ENABLE(DASHBOARD_SUPPORT) was true, and [wv _dashboardBehavior:WebDashboardBehaviorAllowWheelScrolling] always returned true by default (this is due to the underlying value, dashboardBehaviorAllowWheelScrolling in WebViewData.mm being initialized to YES, unlike it's peers, that default initialize to NO).

Great! I’m relieved to hear that.

[Radar WebKit Bug Importer]

<rdar://problem/51606093>