38429 2010-05-02 03:33:55 -0700 Web Inspector: cycle through tag name / attributes / new attribute on Tab. 2010-12-20 23:12:11 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 pfeldman pfeldman bweinstein commit-queue joepeck rik timothy yurys oldest_to_newest 219811 0 pfeldman 2010-05-02 03:33:55 -0700 I am usually adding attributes using following scenario: - Double click on node to start editing random attribute - Press Tab several times to get the 'add attribute' placeholder but sometimes, I am doing an extra click and editing mode vanishes. I think it'd be better to cycle to the edit tag name instead. In the future, we can consider moving to the child nodes / text, but before that is ready, this seems to be a reasonable change. 219813 1 54867 pfeldman 2010-05-02 03:38:24 -0700 Created attachment 54867 [PATCH] Proposed change. 225338 2 joepeck 2010-05-13 09:17:54 -0700 My concerns are that this creates an inconsistency with editing Styles. The patch is simple enough. Have you been running into this problem frequently? 225356 3 pfeldman 2010-05-13 10:08:43 -0700 (In reply to comment #2) > My concerns are that this creates an inconsistency with editing > Styles. The patch is simple enough. Have you been running into this > problem frequently? Try tabbing through the attributes. First you reach new attr placeholder, then editing stops and subsequent tab kicks focus out of dom tree. Now you need mouse... 241031 4 54867 abarth 2010-06-21 18:08:26 -0700 Comment on attachment 54867 [PATCH] Proposed change. This patch has been up for review for a while, but you don't seem to have convinced the inspector front-end folks that this is worth doing. Feel free to renominate for review when you have them onboard. By the way, what is the testing strategy for the inspector front-end code? There don't seem to be any test changes in this patch. 241114 5 54867 pfeldman 2010-06-21 22:42:00 -0700 Comment on attachment 54867 [PATCH] Proposed change. Adam, I don't think we should jump in and start r-ing each-others changes, should we. Or at least please tell me that this game has started and explain the rules so that I was prepared :) Now that Joe self-nominated to review this, I would expect him to do r+/- when he has a minute. You cose a strange way of reminding him about the issue though. Btw, please define "the front-end folks" - you must have some insight into the state of the front-end development I am missing. Ok, enough sarcasm... > By the way, what is the testing strategy for the inspector front-end code? There don't seem to be any test changes in this patch. You are right, we don't have tests for keyboard traversal since they are tricky to implement. In fact, we did not have tests at all a year ago. I think we are doing fair amount of progress in adding them now. Anyways, Joe, I am not insisting on landing this, but I did run into the problem several times when tried adding attributes using keyboard only on elements with > 5 existing attributes. Feel free to r- should you have concerns. I surely can get this reviewed / landed locally, but I'd like to get your Ok given that you've been working on keyboard traversal. 241131 6 joepeck 2010-06-21 23:03:04 -0700 (In reply to comment #5) > (From update of attachment 54867 [details]) > Now that Joe self-nominated to review this, I would expect him to do r+/- > when he has a minute. You c[h]ose a strange way of reminding him about the > issue though. Sorry that I forgot about this! When it was marked as read in my inbox I lost it! I see nothing wrong with the patch, I just felt that it didn't go far enough. I raised a concern on IRC (not logged here). I think that tabbed navigation should be consistent in both the DOM Tree and the Styles Sidebar. What I mean by that is: (1) tabbing in both should cycle or (2) tabbing in both should traverse the entire section. Traversing the entire section would be tag names, attributes, text nodes alike. And through all selectors, properties & values. Would you agree with that, or do you feel this consistency is not as important as I think? I have respect for your front-end decisions, as you have made a number of good decisions based on realistic user scenarios (Edit as HTML for example). > In fact, we did not have tests at all a year ago. I think we are doing > fair amount of progress in adding them now. Very true. > Anyways, Joe, I am not insisting on landing this, but I did run into the > problem several times when tried adding attributes using keyboard > only on elements with > 5 existing attributes. Feel free to r- should > you have concerns. I surely can get this reviewed / landed locally, > but I'd like to get your Ok given that you've been working on keyboard > traversal. This patch in no way detriments the UI, and it can only improve things for users who run into the issue. So I will approve it. One last question. Do you run into the same problem when editing styles? Or is that not the case because you don't have to go through the "tab to create" song and dance because you can just double click to create a new property? 262993 7 54867 abarth 2010-08-10 22:06:10 -0700 Comment on attachment 54867 [PATCH] Proposed change. This patch has been in pending-commit for a while. Shall we commit it? 324794 8 54867 eric 2010-12-20 22:29:07 -0800 Comment on attachment 54867 [PATCH] Proposed change. Going to try at least. :) I think this bug mostly just needs to be closed as abandoned. 324854 9 commit-queue 2010-12-20 23:10:14 -0800 The commit-queue encountered the following flaky tests while processing attachment 54867: inspector/extensions-eval.html bug 51383 (author: caseq@chromium.org) The commit-queue is continuing to process your patch. 324855 10 54867 commit-queue 2010-12-20 23:12:05 -0800 Comment on attachment 54867 [PATCH] Proposed change. Clearing flags on attachment: 54867 Committed r74397: <http://trac.webkit.org/changeset/74397> 324856 11 commit-queue 2010-12-20 23:12:11 -0800 All reviewed patches have been landed. Closing bug. 54867 2010-05-02 03:38:24 -0700 2010-12-20 23:12:05 -0800 [PATCH] Proposed change. cycle_attrs text/plain 1973 pfeldman ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OWYzODE0MC4uNGRlM2YwZSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMC0wNS0wMiAgUGF2ZWwgRmVsZG1h biAgPHBmZWxkbWFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBXZWIgSW5zcGVjdG9yOiBjeWNsZSB0aHJvdWdoIHRhZyBuYW1l IC8gYXR0cmlidXRlcyAvIG5ldyBhdHRyaWJ1dGUgb24gVGFiLgorCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zODQyOQorCisgICAgICAgICogaW5zcGVj dG9yL2Zyb250LWVuZC9FbGVtZW50c1RyZWVPdXRsaW5lLmpzOgorICAgICAgICAoV2ViSW5zcGVj dG9yLkVsZW1lbnRzVHJlZUVsZW1lbnQucHJvdG90eXBlLl90YWdOYW1lRWRpdGluZ0NvbW1pdHRl ZC5tb3ZlVG9OZXh0QXR0cmlidXRlSWZOZWVkZWQpOgorCiAyMDEwLTA0LTI4ICBQYXZlbCBGZWxk bWFuICA8cGZlbGRtYW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFRpbW90 aHkgSGF0Y2hlci4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9FbGVt ZW50c1RyZWVPdXRsaW5lLmpzIGIvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL0VsZW1lbnRz VHJlZU91dGxpbmUuanMKaW5kZXggMWFkZDZjYy4uMDA4ZDlhMCAxMDA2NDQKLS0tIGEvV2ViQ29y ZS9pbnNwZWN0b3IvZnJvbnQtZW5kL0VsZW1lbnRzVHJlZU91dGxpbmUuanMKKysrIGIvV2ViQ29y ZS9pbnNwZWN0b3IvZnJvbnQtZW5kL0VsZW1lbnRzVHJlZU91dGxpbmUuanMKQEAgLTEwMDQsOCAr MTAwNCwxMiBAQCBXZWJJbnNwZWN0b3IuRWxlbWVudHNUcmVlRWxlbWVudC5wcm90b3R5cGUgPSB7 CiAgICAgICAgICAgICBpZiAoIWZvdW5kKSB7CiAgICAgICAgICAgICAgICAgaWYgKG1vdmVEaXJl Y3Rpb24gPT09ICJiYWNrd2FyZCIgJiYgYXR0cmlidXRlcy5sZW5ndGggPiAwKQogICAgICAgICAg ICAgICAgICAgICBtb3ZlVG9BdHRyaWJ1dGUgPSBhdHRyaWJ1dGVzW2F0dHJpYnV0ZXMubGVuZ3Ro IC0gMV0ubmFtZTsKLSAgICAgICAgICAgICAgICBlbHNlIGlmIChtb3ZlRGlyZWN0aW9uID09PSAi Zm9yd2FyZCIgJiYgIS9eXHMqJC8udGVzdChuZXdUZXh0KSkKLSAgICAgICAgICAgICAgICAgICAg bW92ZVRvTmV3QXR0cmlidXRlID0gdHJ1ZTsKKyAgICAgICAgICAgICAgICBlbHNlIGlmIChtb3Zl RGlyZWN0aW9uID09PSAiZm9yd2FyZCIpIHsKKyAgICAgICAgICAgICAgICAgICAgaWYgKCEvXlxz KiQvLnRlc3QobmV3VGV4dCkpCisgICAgICAgICAgICAgICAgICAgICAgICBtb3ZlVG9OZXdBdHRy aWJ1dGUgPSB0cnVlOworICAgICAgICAgICAgICAgICAgICBlbHNlCisgICAgICAgICAgICAgICAg ICAgICAgICBtb3ZlVG9UYWdOYW1lID0gdHJ1ZTsKKyAgICAgICAgICAgICAgICB9CiAgICAgICAg ICAgICB9CiAgICAgICAgIH0KIApAQCAtMTA4Myw4ICsxMDg3LDEwIEBAIFdlYkluc3BlY3Rvci5F bGVtZW50c1RyZWVFbGVtZW50LnByb3RvdHlwZSA9IHsKIAogICAgICAgICBmdW5jdGlvbiBtb3Zl VG9OZXh0QXR0cmlidXRlSWZOZWVkZWQoKQogICAgICAgICB7Ci0gICAgICAgICAgICBpZiAobW92 ZURpcmVjdGlvbiAhPT0gImZvcndhcmQiKQorICAgICAgICAgICAgaWYgKG1vdmVEaXJlY3Rpb24g IT09ICJmb3J3YXJkIikgeworICAgICAgICAgICAgICAgIHRoaXMuX2FkZE5ld0F0dHJpYnV0ZSgp OwogICAgICAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAgICAgIH0KIAogICAgICAgICAgICAg dmFyIGF0dHJpYnV0ZXMgPSB0aGlzLnJlcHJlc2VudGVkT2JqZWN0LmF0dHJpYnV0ZXM7CiAgICAg ICAgICAgICBpZiAoYXR0cmlidXRlcy5sZW5ndGggPiAwKQo=