31049 2009-11-03 01:40:24 -0800 Watch expression editor should stay open after Add button was clicked 2009-11-03 10:45:28 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 yurys webkit-unassigned commit-queue pmuellr timothy oldest_to_newest 160013 0 yurys 2009-11-03 01:40:24 -0800 Steps to reproduce. 1. Go to Scripts panel 2. In "Watch Expression" panel click "Add" Expected result: expression editor is open and focused. Actual result: an empty expression is added. You need to click the colon symbol to start editing. 160024 1 pmuellr 2009-11-03 04:01:30 -0800 Yes, this is a regression. IIRC, the code to make this happen was a bit confusing, as it involved setting the value to some kind of special value. Easy to imagine someone might have "cleaned some code up" but broke it instead. When fixing this problem, let's make sure it's commented well. 160072 2 pmuellr 2009-11-03 08:30:55 -0800 The problem is that the add WatchExpressionsSection.addExpression() was dependent on update() being synchronous. But it's not. I'm curious how this ever worked, unless our eval() code used to be synchronous, but is now asynchronous. Doesn't matter, I have a fix. 160077 3 timothy 2009-11-03 09:01:29 -0800 Yes eval() code used to be synchronous, but is now asynchronous. 160081 4 42387 pmuellr 2009-11-03 09:10:34 -0800 Created attachment 42387 proposed patch 2009/11/03 - a Moved the code to put the entry into "edit" mode into the path where the entry is added to the tree. With the new async eval(), the old code path would never work, because the entry wasn't added by the time the code was run. Because the new check for "edit" mode is now in a code path that runs a lot more often, also added a flag - _newExpressionAdded - to indicate when the check should be made, since it requires a linear search through the watch expressions, and could in theory be expensive. 160083 5 yurys 2009-11-03 09:19:19 -0800 BTW, do we really need to call update and reevaluate all the expressions when a new one is added? 160089 6 pmuellr 2009-11-03 09:33:38 -0800 (In reply to comment #5) > BTW, do we really need to call update and reevaluate all the expressions when a > new one is added? Probably not. OTOH, optimizing to JUST evaluate the new expression will likely add more code, so not clear if it's really worth it, given the probably relative infrequency that people will be updating watch expressions. Part of the problem is also that the WatchExpressions classes subclass ObjectProperties sections, which don't (IIRC) have fine-grained control over dealing with individual members. Hence the kinda hacky findAddedTreeElement() function. Perhaps some additional goodies in the superclasses would make optimizing this a bit easier. Note also that, in general, we don't really update enough. For instance, we don't update after a line is executed on the console, but probably should. You could also imagine updating after every UI event that occurs in the target (well, except some of the excessive mouse events). 160127 7 42387 commit-queue 2009-11-03 10:45:25 -0800 Comment on attachment 42387 proposed patch 2009/11/03 - a Clearing flags on attachment: 42387 Committed r50468: <http://trac.webkit.org/changeset/50468> 160128 8 commit-queue 2009-11-03 10:45:28 -0800 All reviewed patches have been landed. Closing bug. 42387 2009-11-03 09:10:34 -0800 2009-11-03 10:45:25 -0800 proposed patch 2009/11/03 - a 31049.patch text/plain 2607 pmuellr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDQ1NykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMTEtMDMgIFBhdHJpY2sgTXVlbGxlciAgPFBhdHJpY2tfTXVl bGxlckB1cy5pYm0uY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFdhdGNoIGV4cHJlc3Npb24gZWRpdG9yIHNob3VsZCBzdGF5IG9wZW4gYWZ0ZXIg QWRkIGJ1dHRvbiB3YXMgY2xpY2tlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MzEwNDkKKworICAgICAgICBObyBuZXcgdGVzdHMsIHdhcyBhIHJlZ3Jl c3Npb24sIHVzZSBleGlzdGluZyBtYW51YWwgdGVzdC4KKworICAgICAgICAqIGluc3BlY3Rvci9m cm9udC1lbmQvV2F0Y2hFeHByZXNzaW9uc1NpZGViYXJQYW5lLmpzOgorICAgICAgICAoV2ViSW5z cGVjdG9yLldhdGNoRXhwcmVzc2lvbnNTZWN0aW9uLnByb3RvdHlwZS51cGRhdGUpOgorICAgICAg ICAoV2ViSW5zcGVjdG9yLldhdGNoRXhwcmVzc2lvbnNTZWN0aW9uLnByb3RvdHlwZS5hZGRFeHBy ZXNzaW9uKToKKwogMjAwOS0xMS0wMyAgU2ltb24gSGF1c21hbm4gIDxoYXVzbWFubkB3ZWJraXQu b3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFRvciBBcm5lIFZlc3Riw7guCkluZGV4OiBXZWJD b3JlL2luc3BlY3Rvci9mcm9udC1lbmQvV2F0Y2hFeHByZXNzaW9uc1NpZGViYXJQYW5lLmpzCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9XYXRjaEV4cHJlc3Npb25z U2lkZWJhclBhbmUuanMJKHJldmlzaW9uIDUwNDIzKQorKysgV2ViQ29yZS9pbnNwZWN0b3IvZnJv bnQtZW5kL1dhdGNoRXhwcmVzc2lvbnNTaWRlYmFyUGFuZS5qcwkod29ya2luZyBjb3B5KQpAQCAt MTEyLDggKzExMiwxOSBAQCBXZWJJbnNwZWN0b3IuV2F0Y2hFeHByZXNzaW9uc1NlY3Rpb24ucHJv CiAgICAgICAgICAgICAvLyBtZXRob2QgdG8gZ2V0IGFsbCB0aGUgcHJvcGVydGllcyByZWZyZXNo ZWQgYXQgb25jZS4KICAgICAgICAgICAgIHByb3BlcnRpZXMucHVzaChwcm9wZXJ0eSk7CiAgICAg ICAgICAgICAKLSAgICAgICAgICAgIGlmIChwcm9wZXJ0aWVzLmxlbmd0aCA9PSBwcm9wZXJ0eUNv dW50KQorICAgICAgICAgICAgaWYgKHByb3BlcnRpZXMubGVuZ3RoID09IHByb3BlcnR5Q291bnQp IHsKICAgICAgICAgICAgICAgICB0aGlzLnVwZGF0ZVByb3BlcnRpZXMocHJvcGVydGllcywgV2Vi SW5zcGVjdG9yLldhdGNoRXhwcmVzc2lvblRyZWVFbGVtZW50LCBXZWJJbnNwZWN0b3IuV2F0Y2hF eHByZXNzaW9uc1NlY3Rpb24uQ29tcGFyZVByb3BlcnRpZXMpOworCisgICAgICAgICAgICAgICAg Ly8gY2hlY2sgdG8gc2VlIGlmIHdlIGp1c3QgYWRkZWQgYSBuZXcgd2F0Y2ggZXhwcmVzc2lvbiwK KyAgICAgICAgICAgICAgICAvLyB3aGljaCB3aWxsIGFsd2F5cyBiZSB0aGUgbGFzdCBwcm9wZXJ0 eQorICAgICAgICAgICAgICAgIGlmICh0aGlzLl9uZXdFeHByZXNzaW9uQWRkZWQpIHsKKyAgICAg ICAgICAgICAgICAgICAgZGVsZXRlIHRoaXMuX25ld0V4cHJlc3Npb25BZGRlZDsKKworICAgICAg ICAgICAgICAgICAgICB0cmVlRWxlbWVudCA9IHRoaXMuZmluZEFkZGVkVHJlZUVsZW1lbnQoKTsK KyAgICAgICAgICAgICAgICAgICAgaWYgKHRyZWVFbGVtZW50KQorICAgICAgICAgICAgICAgICAg ICAgICAgdHJlZUVsZW1lbnQuc3RhcnRFZGl0aW5nKCk7CisgICAgICAgICAgICAgICAgfQorICAg ICAgICAgICAgfQogICAgICAgICB9CiAKICAgICAgICAgSW5zcGVjdG9yQ29udHJvbGxlci5yZWxl YXNlV3JhcHBlck9iamVjdEdyb3VwKHRoaXMuX3dhdGNoT2JqZWN0R3JvdXBJZCkKQEAgLTE1MCwx NCArMTYxLDkgQEAgV2ViSW5zcGVjdG9yLldhdGNoRXhwcmVzc2lvbnNTZWN0aW9uLnBybwogCiAg ICAgYWRkRXhwcmVzc2lvbjogZnVuY3Rpb24oKQogICAgIHsKKyAgICAgICAgdGhpcy5fbmV3RXhw cmVzc2lvbkFkZGVkID0gdHJ1ZTsKICAgICAgICAgdGhpcy53YXRjaEV4cHJlc3Npb25zLnB1c2go V2ViSW5zcGVjdG9yLldhdGNoRXhwcmVzc2lvbnNTZWN0aW9uLk5ld1dhdGNoRXhwcmVzc2lvbik7 CiAgICAgICAgIHRoaXMudXBkYXRlKCk7Ci0KLSAgICAgICAgLy8gQWZ0ZXIgdXBkYXRlKCksIHRo ZSBuZXcgZW1wdHkgZXhwcmVzc2lvbiB0byBiZSBlZGl0ZWQKLSAgICAgICAgLy8gd2lsbCBiZSBp biB0aGUgdHJlZSwgYnV0IHdlIGhhdmUgdG8gZmluZCBpdC4KLSAgICAgICAgdHJlZUVsZW1lbnQg PSB0aGlzLmZpbmRBZGRlZFRyZWVFbGVtZW50KCk7Ci0gICAgICAgIGlmICh0cmVlRWxlbWVudCkK LSAgICAgICAgICAgIHRyZWVFbGVtZW50LnN0YXJ0RWRpdGluZygpOwogICAgIH0sCiAKICAgICB1 cGRhdGVFeHByZXNzaW9uOiBmdW5jdGlvbihlbGVtZW50LCB2YWx1ZSkK