200564 2019-08-08 22:53:37 -0700 REGRESSION(r248391): Web Inspector: changing Layout Direction Debug setting no longer adds dir="ltr" to body element 2019-08-12 20:57:20 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 nvasilyev hi commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1559806 0 nvasilyev 2019-08-08 22:53:37 -0700 This has to be recent. 1559807 1 nvasilyev 2019-08-08 23:04:14 -0700 WI.resolvedLayoutDirection = function() { let layoutDirection = WI.settings.debugLayoutDirection.value; -> if (!WI.isDebugUIEnabled() || layoutDirection === WI.LayoutDirection.System) layoutDirection = InspectorFrontendHost.userInterfaceLayoutDirection(); return layoutDirection; }; WI.isDebugUIEnabled returns undefined: WI.isDebugUIEnabled = function() { return WI.showDebugUISetting && WI.showDebugUISetting.value; }; WI.showDebugUISetting is undefined. This is where it's created: // This function is invoked after the inspector has loaded and has a backend target. WI.runBootstrapOperations = function() { WI.showDebugUISetting = new WI.Setting("show-debug-ui", false); 1559808 2 nvasilyev 2019-08-08 23:08:39 -0700 Broke in https://trac.webkit.org/changeset/248391/webkit#file1 1559810 3 nvasilyev 2019-08-08 23:15:06 -0700 This line broke it. Main.js: - 2707 if (layoutDirection === WI.LayoutDirection.System) + 2707 if (!WI.isDebugUIEnabled() || layoutDirection === WI.LayoutDirection.System) 1559818 4 hi 2019-08-08 23:36:44 -0700 This is something we don't want to expose outside of Web Inspector's debug mode (⌥⇧⌘D). Are you saying that the layout direction doesn't work when NOT in the debug mode (⌥⇧⌘D)? 1559823 5 nvasilyev 2019-08-09 00:01:23 -0700 (In reply to Devin Rousso from comment #4) > This is something we don't want to expose outside of Web Inspector's debug > mode (⌥⇧⌘D). > > Are you saying that the layout direction doesn't work when NOT in the debug > mode (⌥⇧⌘D)? It doesn't work in Debug mode. 1559825 6 375897 hi 2019-08-09 00:18:15 -0700 Created attachment 375897 Patch 1559832 7 nvasilyev 2019-08-09 02:02:19 -0700 This indeed fixes the regression. I'd r+. Nit: I noticed that several settings are instantiated in WI.loaded() while none in Bootstrap.js. Would it be better to add this one to WI.loaded() as well for consistency? 1559914 8 hi 2019-08-09 09:32:39 -0700 (In reply to Nikita Vasilyev from comment #7) > Nit: I noticed that several settings are instantiated in WI.loaded() while none in Bootstrap.js. Would it be better to add this one to WI.loaded() as well for consistency? `WI.loaded` exists just as a way to say "run this stuff once all the scripts have loaded, but potentially before we're done creating the DOM". We primarily use it now for initial state setup (both UI and protocol). Frankly, we could probably just inline all of that code inside the last script in Main.html/Test.html (or distribute it, as the comment in AppControllerBase.js suggests). In the case of `WI.showDebugUISetting`, most of our other global state `WI.Setting` are created in Setting.js itself (e.g. when the script is parsed/executed, not in some event), I think it's fine to do a similar thing in Bootstrap.js. Putting it in `WI.loaded` would have the same effect, just delayed slightly more. 1560678 9 375897 joepeck 2019-08-12 20:46:09 -0700 Comment on attachment 375897 Patch rs=me 1560690 10 375897 commit-queue 2019-08-12 20:56:44 -0700 Comment on attachment 375897 Patch Clearing flags on attachment: 375897 Committed r248590: <https://trac.webkit.org/changeset/248590> 1560691 11 commit-queue 2019-08-12 20:56:45 -0700 All reviewed patches have been landed. Closing bug. 1560692 12 webkit-bug-importer 2019-08-12 20:57:20 -0700 <rdar://problem/54240508> 375897 2019-08-09 00:18:15 -0700 2019-08-12 20:56:44 -0700 Patch bug-200564-20190809001814.patch text/plain 2455 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGNlMzJhYzFmZmFkZDY5MjY5MGZjYmY3Y2NjN2Vi NTViZGI1ZGY1NjcuLmYwZmM4ZDIzMDRhNzUwZjJiMTM1ODM2ZmE1MGVlYjI5NTFjODdkYzcgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl Ykluc3BlY3RvclVJL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDE5LTA4LTA5ICBEZXZp biBSb3Vzc28gIDxkcm91c3NvQGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OKHIyNDgz OTEpOiBXZWIgSW5zcGVjdG9yOiBjaGFuZ2luZyBMYXlvdXQgRGlyZWN0aW9uIERlYnVnIHNldHRp bmcgbm8gbG9uZ2VyIGFkZHMgZGlyPSJsdHIiIHRvIGJvZHkgZWxlbWVudAorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjAwNTY0CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgYFdJLnJlc29sdmVkTGF5b3V0RGly ZWN0aW9uYCB3YXMgY2FsbGVkIGJlZm9yZSBgV0kucnVuQm9vdHN0cmFwT3BlcmF0aW9uc2AsIHdo aWNoIGlzIHdoYXQKKyAgICAgICAgaW5zdGFudGlhdGVzIGBXSS5zaG93RGVidWdVSVNldHRpbmdg LiBXaXRob3V0IGl0LCBgV0kucmVzb2x2ZWRMYXlvdXREaXJlY3Rpb25gIHdpbGwgaWdub3JlCisg ICAgICAgIHRoZSB2YWx1ZSBvZiBgV0kuc2V0dGluZ3MuZGVidWdMYXlvdXREaXJlY3Rpb25gIGFu ZCBpbnN0ZWFkIHVzZSB0aGUgc3lzdGVtLgorCisgICAgICAgIE1vdmluZyB0aGUgaW5zdGFudGlh dGlvbiBvZiBgV0kuc2hvd0RlYnVnVUlTZXR0aW5nYCBvdXRzaWRlIGBXSS5ydW5Cb290c3RyYXBP cGVyYXRpb25zYAorICAgICAgICBhbGxvd3MgdGhlIHNldHRpbmcgdG8gYmUgY3JlYXRlZCB3aGVu IHRoZSBCb290c3RyYXAuanMgc2NyaXB0IGlzIGxvYWRlZCwgcmF0aGVyIHRoYW4gYWZ0ZXIKKyAg ICAgICAgdGhlIGBET01Db250ZW50TG9hZGVkYCBldmVudCBpcyBmaXJlZC4gVGhpcyBtZWFucyB0 aGF0IGl0J3MgZ3VhcmFudGVlZCB0byBleGlzdCBiZWZvcmUgYW55CisgICAgICAgIGludGVyZmFj ZS92aWV3IGNvZGUgcnVucy4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvRGVidWcvQm9vdHN0 cmFwLmpzOgorICAgICAgICAoV0kucnVuQm9vdHN0cmFwT3BlcmF0aW9ucyk6CisKIDIwMTktMDgt MDggIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNw ZWN0b3I6IFBhZ2U6IGRvbid0IGFsbG93IHRoZSBkb21haW4gdG8gYmUgZGlzYWJsZWQKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL0RlYnVnL0Jvb3RzdHJh cC5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL0RlYnVnL0Jvb3RzdHJh cC5qcwppbmRleCBmNTgzNGQxOTJiMzMzYzI2ZjYxZjQxNDAxMzMwNDU0YTkwZjUzOTdlLi43MmVj MmY3NTAxNTAyNTE4NDE4NjkxZWNhM2ViNzY1Y2RlYmE1ZjY2IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9EZWJ1Zy9Cb290c3RyYXAuanMKKysrIGIvU291 cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvRGVidWcvQm9vdHN0cmFwLmpzCkBAIC0z NywxMCArMzcsMTAgQEAgV0kuc2V0dGluZ3MuZW5naW5lZXJpbmdQYXVzZUZvckludGVybmFsU2Ny aXB0cy5hZGRFdmVudExpc3RlbmVyKFdJLlNldHRpbmcuRXZlbnQKICAgICAgICAgV0kuc2V0dGlu Z3MuZW5naW5lZXJpbmdTaG93SW50ZXJuYWxTY3JpcHRzLnZhbHVlID0gdHJ1ZTsKIH0sIFdJLnNl dHRpbmdzLmVuZ2luZWVyaW5nU2hvd0ludGVybmFsU2NyaXB0cyk7CiAKK1dJLnNob3dEZWJ1Z1VJ U2V0dGluZyA9IG5ldyBXSS5TZXR0aW5nKCJzaG93LWRlYnVnLXVpIiwgZmFsc2UpOworCiAvLyBU aGlzIGZ1bmN0aW9uIGlzIGludm9rZWQgYWZ0ZXIgdGhlIGluc3BlY3RvciBoYXMgbG9hZGVkIGFu ZCBoYXMgYSBiYWNrZW5kIHRhcmdldC4KIFdJLnJ1bkJvb3RzdHJhcE9wZXJhdGlvbnMgPSBmdW5j dGlvbigpIHsKLSAgICBXSS5zaG93RGVidWdVSVNldHRpbmcgPSBuZXcgV0kuU2V0dGluZygic2hv dy1kZWJ1Zy11aSIsIGZhbHNlKTsKLQogICAgIC8vIFRvZ2dsZSBEZWJ1ZyBVSSBzZXR0aW5nLgog ICAgIG5ldyBXSS5LZXlib2FyZFNob3J0Y3V0KFdJLktleWJvYXJkU2hvcnRjdXQuTW9kaWZpZXIu T3B0aW9uIHwgV0kuS2V5Ym9hcmRTaG9ydGN1dC5Nb2RpZmllci5TaGlmdCB8IFdJLktleWJvYXJk U2hvcnRjdXQuTW9kaWZpZXIuQ29tbWFuZE9yQ29udHJvbCwgIkQiLCAoKSA9PiB7CiAgICAgICAg IFdJLnNob3dEZWJ1Z1VJU2V0dGluZy52YWx1ZSA9ICFXSS5zaG93RGVidWdVSVNldHRpbmcudmFs dWU7Cg==