168120 2017-02-10 09:42:21 -0800 REGRESSION(r188138): Web Inspector: cannot edit next/previous DOM attribute by using tab/shift-tab 2017-02-13 11:16:27 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 bburg mattbaker commit-queue hi inspector-bugzilla-changes mattbaker webkit-bug-importer oldest_to_newest 1275351 0 bburg 2017-02-10 09:42:21 -0800 This is definitely a regression. It used to be possible to tab through attributes, which would cancel the current edit and start editing the next attribute. Steps to reproduce: 1. Inspect an element with >1 attribute in Elements Tab 2. Press 'Enter' to start editing the first attribute 3. Press 'Tab' to cancel editing of first attribute and start editing next attribute => DOMTreeOutline loses selection, sometimes the Quick Console gets caret/focus 1275352 1 webkit-bug-importer 2017-02-10 09:42:42 -0800 <rdar://problem/30466065> 1275771 2 mattbaker 2017-02-11 01:50:00 -0800 Regressed way back in https://trac.webkit.org/changeset/188138. Tabbing to the next attribute *does* work, as long as the current attribute text changes: --- a/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js +++ b/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js @@ -874,6 +875,9 @@ WebInspector.DOMTreeElement = class DOMTreeElement extends WebInspector.TreeElem _attributeEditingCommitted(element, newText, oldText, attributeName, moveDirection) { + if (newText === oldText) + return; + this._editing = false; var treeOutline = this.treeOutline; 1275772 3 301249 mattbaker 2017-02-11 01:58:05 -0800 Created attachment 301249 Patch 1276098 4 301249 joepeck 2017-02-12 22:27:38 -0800 Comment on attachment 301249 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301249&action=review > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:963 > + if (!moveDirection && newText === oldText) What about all of the other functions with moveDirection? _tagNameEditingCommitted and _attributeNumberEditingCommitted? 1276129 5 301249 mattbaker 2017-02-13 00:44:11 -0800 Comment on attachment 301249 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301249&action=review >> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:963 >> + if (!moveDirection && newText === oldText) > > What about all of the other functions with moveDirection? _tagNameEditingCommitted and _attributeNumberEditingCommitted? I didn't notice these. However, it looks like _tagNameEditingCommitted handles moveDirection in `cancel()`, and _attributeNumberEditingCommitted ignores the parameter. 1276259 6 301249 commit-queue 2017-02-13 11:16:23 -0800 Comment on attachment 301249 Patch Clearing flags on attachment: 301249 Committed r212236: <http://trac.webkit.org/changeset/212236> 1276260 7 commit-queue 2017-02-13 11:16:27 -0800 All reviewed patches have been landed. Closing bug. 301249 2017-02-11 01:58:05 -0800 2017-02-13 11:16:23 -0800 Patch bug-168120-20170211015537.patch text/plain 1572 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjEyMTc3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCA1 ZjRhZGJkZjMxYmE0YWVkY2I0NzdjZDBiMTM1NWFkNmFlYTRlM2YwLi5mMzI0ODFiMTg4ZDI1NWMz ZDI0ZGNhYmY2N2JkY2MxOTIwMTBlNzRkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxNy0wMi0xMSAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgUkVHUkVTU0lPTihyMTg4MTM4KTogV2ViIEluc3BlY3RvcjogY2Fubm90IGVk aXQgbmV4dC9wcmV2aW91cyBET00gYXR0cmlidXRlIGJ5IHVzaW5nIHRhYi9zaGlmdC10YWIKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2ODEyMAorICAg ICAgICA8cmRhcjovL3Byb2JsZW0vMzA0NjYwNjU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgRE9NVHJlZUVsZW1lbnQncyBhdHRyaWJ1dGUgZWRpdGlu ZyBjb21taXR0ZWQgaGFuZGxlciBzaG91bGRuJ3QgZWFybHkgcmV0dXJuCisgICAgICAgIHdoZW4g YSBgbW92ZURpcmVjdGlvbmAgaGFzIGJlZW4gc3BlY2lmaWVkLgorCisgICAgICAgICogVXNlcklu dGVyZmFjZS9WaWV3cy9ET01UcmVlRWxlbWVudC5qczoKKwogMjAxNy0wMi0xMCAgTWF0dCBCYWtl ciAgPG1hdHRiYWtlckBhcHBsZS5jb20+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogRGVidWdn ZXIgc2lkZWJhciBwYW5lbCBzaG91bGQgbm90IGhhdmUgbXVsdGlwbGUgdHJlZSBzZWxlY3Rpb25z CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9E T01UcmVlRWxlbWVudC5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1Zp ZXdzL0RPTVRyZWVFbGVtZW50LmpzCmluZGV4IGI3NmMzZDRkNjkzOGExYzFiNDE5NDI1NmMzOGM0 MzJiZWUwMmMxMGYuLmJlZTdhMWQ2N2ZhNzVkYjIxMzc3ZjlkZjkyZGUzNzA3M2EzM2ZmNmUgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0RPTVRy ZWVFbGVtZW50LmpzCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1Zp ZXdzL0RPTVRyZWVFbGVtZW50LmpzCkBAIC05NjAsNyArOTYwLDcgQEAgV2ViSW5zcGVjdG9yLkRP TVRyZWVFbGVtZW50ID0gY2xhc3MgRE9NVHJlZUVsZW1lbnQgZXh0ZW5kcyBXZWJJbnNwZWN0b3Iu VHJlZUVsZW0KICAgICB7CiAgICAgICAgIHRoaXMuX2VkaXRpbmcgPSBmYWxzZTsKIAotICAgICAg ICBpZiAobmV3VGV4dCA9PT0gb2xkVGV4dCkKKyAgICAgICAgaWYgKCFtb3ZlRGlyZWN0aW9uICYm IG5ld1RleHQgPT09IG9sZFRleHQpCiAgICAgICAgICAgICByZXR1cm47CiAKICAgICAgICAgdmFy IHRyZWVPdXRsaW5lID0gdGhpcy50cmVlT3V0bGluZTsK