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.
Created attachment 54867 [details] [PATCH] Proposed change.
My concerns are that this creates an inconsistency with editing Styles. The patch is simple enough. Have you been running into this problem frequently?
(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...
Comment on attachment 54867 [details] [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.
Comment on attachment 54867 [details] [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.
(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?
Comment on attachment 54867 [details] [PATCH] Proposed change. This patch has been in pending-commit for a while. Shall we commit it?
Comment on attachment 54867 [details] [PATCH] Proposed change. Going to try at least. :) I think this bug mostly just needs to be closed as abandoned.
The commit-queue encountered the following flaky tests while processing attachment 54867 [details]: inspector/extensions-eval.html bug 51383 (author: caseq@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 54867 [details] [PATCH] Proposed change. Clearing flags on attachment: 54867 Committed r74397: <http://trac.webkit.org/changeset/74397>
All reviewed patches have been landed. Closing bug.