Bug 159383 - [RTL Scrollbars] RTL Scrollbars broken with clients creating Web Views via [WKView initWithFrame:contextRef:pageGroupRef:]
Summary: [RTL Scrollbars] RTL Scrollbars broken with clients creating Web Views via [W...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-07-01 19:36 PDT by Myles C. Maxfield
Modified: 2016-07-06 14:23 PDT (History)
3 users (show)

See Also:

Attachments
Patch (3.08 KB, patch)
2016-07-01 19:44 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Patch (3.11 KB, patch)
2016-07-06 11:59 PDT, Myles C. Maxfield 		andersca: review+
Details | Formatted Diff | Diff
Patch for committing (3.11 KB, patch)
2016-07-06 12:05 PDT, Myles C. Maxfield 		commit-queue: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2016-07-01 19:36:21 PDT 
[RTL Scrollbars] RTL Scrollbars broken with clients creating Web Views via [WKView initWithFrame:contextRef:pageGroupRef:]
Comment 1 Myles C. Maxfield 2016-07-01 19:44:34 PDT 
Created attachment 282633 [details]
Patch
Comment 2 Myles C. Maxfield 2016-07-01 19:45:08 PDT 
<rdar://problem/26921117>
Comment 3 Anders Carlsson 2016-07-03 07:45:16 PDT 
Comment on attachment 282633 [details]
Patch

I think this is going to break the case where you have the policy set on the preferences?
Comment 4 Myles C. Maxfield 2016-07-03 09:24:48 PDT 
(In reply to comment #3)
> Comment on attachment 282633 [details]
> Patch
> 
> I think this is going to break the case where you have the policy set on the
> preferences?

There are two bits:
Whether the policy is .Content or .System
Whether or not the system is in RTL

This patch only sets the second bit. The second bit is only consulted if the policy is .System.
Comment 5 Anders Carlsson 2016-07-06 11:53:26 PDT 
Comment on attachment 282633 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:992
> +static uint32_t convertSystemLayoutDirection(NSUserInterfaceLayoutDirection direction)

I'd change this to return the WebCore::UserInterfaceLayoutDirection and then to the uint32_t cast at the call site.
Comment 6 Myles C. Maxfield 2016-07-06 11:59:16 PDT 
Created attachment 282919 [details]
Patch
Comment 7 Anders Carlsson 2016-07-06 12:04:04 PDT 
Comment on attachment 282919 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:992
> +static WebCore::UserInterfaceLayoutDirection convertSystemLayoutDirection(NSUserInterfaceLayoutDirection direction)

I would call this toUserInterfaceLayoutDirection - I think we use that idiom in other places.
Comment 8 Myles C. Maxfield 2016-07-06 12:05:57 PDT 
Created attachment 282922 [details]
Patch for committing
Comment 9 WebKit Commit Bot 2016-07-06 13:03:12 PDT 
Comment on attachment 282922 [details]
Patch for committing

Rejecting attachment 282922 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 282922, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebKit2/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/1637210
Comment 10 Myles C. Maxfield 2016-07-06 14:23:50 PDT 
Committed r202878: <http://trac.webkit.org/changeset/202878>