Bug 168273

Summary: Web Inspector: RTL: Inspector window should dock to the left when using RTL layout direction
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, bburg, commit-queue, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Screen recording - RTL and LTR dock to side
none
Patch v2 mattbaker: review+, commit-queue: commit-queue-

Description BJ Burg 2017-02-13 20:59:50 PST
<rdar://problem/29949325>
Comment 1 BJ Burg 2017-02-17 13:08:00 PST
Created attachment 301975 [details]
Patch
Comment 2 BJ Burg 2017-02-17 13:17:09 PST
Created attachment 301978 [details]
Screen recording - RTL and LTR dock to side

Things Tested:

- Opens docked to left with correct dimensions
- Can drag bigger and smaller
- Won't drag if cursor goes beyond the resizer due to min/max size
- inspected and inspector views have correct frames when changing dock side
- changing layout direction will flip dock side if necessary
- Dock to Side button has layout direction-based icon and behavior
Comment 3 Matt Baker 2017-02-17 13:38:19 PST
Comment on attachment 301975 [details]
Patch

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

Clicking

> Source/WebInspectorUI/UserInterface/Base/Main.js:365
> +    let dockToSideCallback = WebInspector.resolvedLayoutDirection() === WebInspector.LayoutDirection.RTL ? this._dockLeft : this._dockRight;

Why not just use a single callback, which checks WebInspector.resolvedLayoutDirection()?

> Source/WebInspectorUI/UserInterface/Base/Main.js:1585
> +WebInspector._dockLeft = function(event)

See comment above:

WebInspector._dockSide = function(event)
{
    let dockSide = WebInspector.resolvedLayoutDirection() === WebInspector.LayoutDirection.RTL ? WebInspector.DockConfiguration.Left : WebInspector.DockConfiguration.Right;
    InspectorFrontendHost.requestSetDockSide(dockSide);
};
Comment 4 Matt Baker 2017-02-17 13:52:44 PST
Comment on attachment 301975 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Base/Main.js:2186
> +        this._dockLeft();

Oops! Please disregard previous comment regarding `_dockSide`.

> Source/WebInspectorUI/UserInterface/Views/Main.css:63
> +body.docked.right {

Should be `body.docked.left`

> Source/WebInspectorUI/UserInterface/Views/Main.css:99
> +    top: 0;

Nit: position and top can be moved to the `body.docked #docked-resizer` rule.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:574
> +        CGFloat inspectorWidth = InspectorFrontendClientLocal::constrainedAttachedWindowWidth(currentDimension, parentWidth);

Nit: add a newline to be consistent with previous case.
Comment 5 BJ Burg 2017-02-17 14:29:38 PST
Created attachment 301989 [details]
Patch v2
Comment 6 Matt Baker 2017-02-17 14:49:51 PST
Comment on attachment 301989 [details]
Patch v2

r=me
Comment 7 WebKit Commit Bot 2017-02-17 15:15:14 PST
Comment on attachment 301989 [details]
Patch v2

Rejecting attachment 301989 [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-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 301989, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
    -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 212580 = 7ef2ceefa819efca36ab89fbad9450c8d691ca1f
r212581 = eb489244b684d49c487e3c463b534d5fb7c4ec55
r212582 = b3540266eaebafedf8ed23a3be655a616f881b98
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/3145626
Comment 8 BJ Burg 2017-02-17 16:42:08 PST
Committed r212597: <http://trac.webkit.org/changeset/212597>