Bug 171402

Summary: Web Inspector: RTL: inherit system layout direction by default
Product: WebKit Reporter: Blaze Burg <bburg>
Component: Web InspectorAssignee: Blaze Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, dbates, inspector-bugzilla-changes, joepeck, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Blaze Burg
Reported 2017-04-27 16:14:46 PDT
Ready, set, file bugs!
Attachments
Patch (11.30 KB, patch)
2017-04-27 16:28 PDT, Blaze Burg
no flags
Patch (9.55 KB, patch)
2017-04-27 16:39 PDT, Blaze Burg
no flags
Blaze Burg
Comment 1 2017-04-27 16:15:13 PDT
Blaze Burg
Comment 2 2017-04-27 16:28:42 PDT
Joseph Pecoraro
Comment 3 2017-04-27 16:38:45 PDT
Comment on attachment 308473 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308473&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Setting.js:115 > - // FIXME: change initial value to 'system' once we are happy with RTL support. > - // This will cause Web Inspector to use the system user interface layout direction. > - layoutDirection: new WebInspector.Setting("layout-direction", "ltr"), > + layoutDirectionOverride: new WebInspector.Setting("layout-direction-override", "system"), I realize the storage name needs to change in order for us to get the new default value of "system" going forward. However I think WebInspector.settings.layoutDirection is way better than WebInspector.settings.layoutDirectionOverride. I suggest we keep that. This brings to light that in the future we should name settings that are in experimental stages with a storage name "...-experimental" so that when we turn it on by default and rename it we can get the better name when it graduates!
Blaze Burg
Comment 4 2017-04-27 16:39:37 PDT
Blaze Burg
Comment 5 2017-04-27 16:41:10 PDT
(In reply to Joseph Pecoraro from comment #3) > Comment on attachment 308473 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=308473&action=review > > r=me > > > Source/WebInspectorUI/UserInterface/Base/Setting.js:115 > > - // FIXME: change initial value to 'system' once we are happy with RTL support. > > - // This will cause Web Inspector to use the system user interface layout direction. > > - layoutDirection: new WebInspector.Setting("layout-direction", "ltr"), > > + layoutDirectionOverride: new WebInspector.Setting("layout-direction-override", "system"), > > I realize the storage name needs to change in order for us to get the new > default value of "system" going forward. > > However I think WebInspector.settings.layoutDirection is way better than > WebInspector.settings.layoutDirectionOverride. I suggest we keep that. > > This brings to light that in the future we should name settings that are in > experimental stages with a storage name "...-experimental" so that when we > turn it on by default and rename it we can get the better name when it > graduates! OK!
Blaze Burg
Comment 6 2017-04-27 16:45:00 PDT
Matt Baker
Comment 7 2017-05-01 16:28:07 PDT
(In reply to Joseph Pecoraro from comment #3) > This brings to light that in the future we should name settings that are in > experimental stages with a storage name "...-experimental" so that when we > turn it on by default and rename it we can get the better name when it > graduates! We could either: 1) Create an ExperimentalSetting class which adds the storage name. 2) Add an `experimental` property (default false) to Setting which does the same thing.
Daniel Bates
Comment 8 2018-04-29 22:53:48 PDT
Comment on attachment 308474 [details] Patch Clearing review flag as this patch was landed per comment 6.
Note You need to log in before you can comment on or make changes to this bug.