Bug 183166 – [Win][DRT] Implement setSpatialNavigationEnabled.

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

[Ross Kirsling]

[WinCairo][DRT] Implement setSpatialNavigationEnabled.

[Ross Kirsling]

Created attachment 334697 [details]
Patch

[Ross Kirsling]

I'm just focusing on WinCairo here, but let me know if you'd like me to remove the Skip line from AppleWin TestExpectations as well:
https://github.com/WebKit/webkit/blob/master/LayoutTests/platform/win/TestExpectations#L228-L229

[Ross Kirsling]

Created attachment 334704 [details]
Patch

[Ross Kirsling]

Comment on attachment 334704 [details]
Patch

...figured if I was going to bother to mention it, I should just go ahead and do it.

[Ross Kirsling]

The WinCairo failure here is a CMake fluke (no code was changed from the previous patch); I've made a bug for it here:
https://bugs.webkit.org/show_bug.cgi?id=183177

[EWS Watchlist]

Comment on attachment 334704 [details]
Patch

Attachment 334704 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6695928

New failing tests:
fast/spatial-navigation/snav-multiple-select-focusring.html

[EWS Watchlist]

Created attachment 334707 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Ross Kirsling]

Created attachment 334708 [details]
Patch

[Per Arne Vollan]

Comment on attachment 334708 [details]
Patch

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

> Source/WebKitLegacy/win/Interfaces/IWebPreferencesPrivate.idl:123
> +    HRESULT isSpatialNavigationEnabled([out, retval] BOOL *enabled);
> +    HRESULT setSpatialNavigationEnabled([in] BOOL enabled);

I think you need to add this to the end of IWebPreferencesPrivate6, or create IWebPreferencesPrivate7, and add these methods there.

[Ross Kirsling]

Created attachment 334754 [details]
Patch

[Ross Kirsling]

Created attachment 334765 [details]
Patch

[Ross Kirsling]

Thanks, I think this should be correct now.

[Per Arne Vollan]

Comment on attachment 334765 [details]
Patch

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

> Source/WebKitLegacy/win/Interfaces/IWebPreferencesPrivate.idl:224
> +    HRESULT isSpatialNavigationEnabled([out, retval] BOOL *enabled);

I think we usually would use 'spatialNavigationEnabled()' here.

[Ross Kirsling]

(In reply to Per Arne Vollan from comment #13)
> Comment on attachment 334765 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334765&action=review
> 
> > Source/WebKitLegacy/win/Interfaces/IWebPreferencesPrivate.idl:224
> > +    HRESULT isSpatialNavigationEnabled([out, retval] BOOL *enabled);
> 
> I think we usually would use 'spatialNavigationEnabled()' here.

I was just mimicking "isXSSAuditorEnabled", but I'm happy to do either.

[Ross Kirsling]

Created attachment 334828 [details]
Patch

[Per Arne Vollan]

Comment on attachment 334828 [details]
Patch

R=me as long as ews is green.

[Don Olmstead]

(In reply to Per Arne Vollan from comment #16)
> Comment on attachment 334828 [details]
> Patch
> 
> R=me as long as ews is green.

On WinCairo we seem to be hitting https://bugs.webkit.org/show_bug.cgi?id=183177 with this patch. We're fine with landing it as is.

[WebKit Commit Bot]

Comment on attachment 334828 [details]
Patch

Clearing flags on attachment: 334828

Committed r229143: <https://trac.webkit.org/changeset/229143>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38039052>