Bug 31049 – Watch expression editor should stay open after Add button was clicked

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Yury Semikhatsky]

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.

[Patrick Mueller]

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.

[Patrick Mueller]

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.

[Timothy Hatcher]

Yes eval() code used to be synchronous, but is now asynchronous.

[Patrick Mueller]

Created attachment 42387 [details]
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.

[Yury Semikhatsky]

BTW, do we really need to call update and reevaluate all the expressions when a new one is added?

[Patrick Mueller]

(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).

[WebKit Commit Bot]

Comment on attachment 42387 [details]
proposed patch 2009/11/03 - a

Clearing flags on attachment: 42387

Committed r50468: <http://trac.webkit.org/changeset/50468>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.