Bug 44957 - Web Inspector, Extension API: Panel.onSelectionChanged event is never fired
Summary: Web Inspector, Extension API: Panel.onSelectionChanged event is never fired
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-31 09:07 PDT by Andrey Kosyakov
Modified: 2010-09-14 00:25 PDT (History)
5 users (show)

See Also:


Attachments
patch (6.48 KB, patch)
2010-08-31 09:24 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
patch (3.10 KB, patch)
2010-08-31 10:39 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
patch (6.87 KB, patch)
2010-08-31 10:48 PDT, Andrey Kosyakov
yurys: review-
Details | Formatted Diff | Diff
patch (12.85 KB, patch)
2010-09-01 06:04 PDT, Andrey Kosyakov
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2010-08-31 09:07:08 PDT
webInspector.panels.elements.onSelectionChanged should be fired when a DOM element is selected in elements panel.
Comment 1 Andrey Kosyakov 2010-08-31 09:24:11 PDT
Created attachment 66064 [details]
patch

- Added Panel.name
- Fixed the way onSelectionChanged is dispatched from ElementsPanel.js
- Added console API into context of code being evaluated by webInspector.inspectedWindow.evaluate() (so we can do inspect() there)
- Added basic test for extension API events
Comment 2 Andrey Kosyakov 2010-08-31 10:39:54 PDT
Created attachment 66071 [details]
patch

- Extracted stringifying wrapper around expressions evaluated by extensions into a more readable function.
Comment 3 Andrey Kosyakov 2010-08-31 10:45:58 PDT
Comment on attachment 66071 [details]
patch

oops, wrong patch. sorry.
Comment 4 Andrey Kosyakov 2010-08-31 10:48:50 PDT
Created attachment 66073 [details]
patch
Comment 5 Yury Semikhatsky 2010-09-01 01:48:24 PDT
Comment on attachment 66073 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66073&action=prettypatch

> WebCore/inspector/front-end/ExtensionServer.js:225
> +            var result = window.eval(
I believe we can get rid of this eval by providing the function body directly or even call the eval without wrapping it into an anonymous function as we discussed offline. A bigger problem is that this code doesn't allow extensions to declare global functions, r- for this.
Comment 6 Andrey Kosyakov 2010-09-01 06:04:24 PDT
Created attachment 66207 [details]
patch

- changed webInspector.inspectedWindow.evaluaet() to return object instead of its stringified representation
- rewrote wrapper for evaluated code
- added more tests for eval
Comment 7 Andrey Kosyakov 2010-09-01 06:06:01 PDT
(In reply to comment #5)
> (From update of attachment 66073 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=66073&action=prettypatch
> 
> > WebCore/inspector/front-end/ExtensionServer.js:225
> > +            var result = window.eval(
> I believe we can get rid of this eval by providing the function body directly or even call the eval without wrapping it into an anonymous function as we discussed offline. A bigger problem is that this code doesn't allow extensions to declare global functions, r- for this.

The latter happened to be not the case: the code being evaluated still runs in global context. Added a test for that, though, and rewrote wrapper code to a shorter (though less readable) version without outer function.
Comment 8 Andrey Kosyakov 2010-09-01 08:48:50 PDT
Manually committed r66601: http://trac.webkit.org/changeset/66601
Comment 9 Eric Seidel (no email) 2010-09-13 21:28:32 PDT
http://trac.webkit.org/browser/trunk/LayoutTests/inspector/extensions-events.html is flaky on the Leopard Commit bot and is causing the bot to back up. :(
Comment 10 Eric Seidel (no email) 2010-09-13 21:29:30 PDT
1  CONSOLE MESSAGE: line 663: [object HTMLParagraphElement]
 1 CONSOLE MESSAGE: line 667: [object HTMLParagraphElement]

Is the diff.
Comment 11 Andrey Kosyakov 2010-09-14 00:25:53 PDT
(In reply to comment #10)
> 1  CONSOLE MESSAGE: line 663: [object HTMLParagraphElement]
>  1 CONSOLE MESSAGE: line 667: [object HTMLParagraphElement]
> 
> Is the diff.

This should have been fixed by r67394: http://trac.webkit.org/changeset/67394
It is not actual flakiness, rather a stable, though unexpected, side effect from r67385: the inspect() console command line API call uses console.log() internally, which causes line number of a line within InjectedScript.js that called console.log() to appear in test expectations. Hence a change to InjectedScripts.js that causes a call to console.log() to move causes test expectations change.