Closed Bug 636169 Opened 9 years ago Closed 9 years ago

Web Console notifications needed for customization via Addons

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 4.0

People

(Reporter: ddahl, Assigned: ddahl)

References

Details

(Whiteboard: [approved-patches-landed])

Attachments

(3 files, 4 obsolete files)

We will need a couple of notifications for addons and other web console code to listen for to make the web console customizable.

I'm thinking web-console-created, web-console-destroyed, web-console-collapsed and web-console-opened
Assignee: nobody → ddahl
a couple off the top of my stack:

web-console-filters-changed
web-console-cleared
js-term-executed
js-term-helper-executed
web-console-message-created

... anything else?

what about a web-console-page-console-modified for when the window.console object gets paved over or modified?
(In reply to comment #1)
> a couple off the top of my stack:
> 
> web-console-filters-changed
> web-console-cleared
> js-term-executed
> js-term-helper-executed
> web-console-message-created
> 
> ... anything else?
> 
> what about a web-console-page-console-modified for when the window.console
> object gets paved over or modified?

I like these, i wonder what the reviewer will think about so many...
Summary: Console notifications needed for customization → Web Console notifications needed for customization via Addons
Attached patch v 0.1added several notifications (obsolete) — Splinter Review
Added several notifications:

web-console-created
web-console-destroyed
web-console-page-console-modified
web-console-message-created

- the concept is that the DevTools SDK will provide a registry of events that can have callbacks - the callback will then be handed a simplified WebConsole object with an elegant API exposing only what needs to be exposed to Addon devs. It will be trivial to let addons know when a new console is opened so it can be modified, e.g:

DevToolsSDK.webconsole.onCreate(function(aWebConsoleWrapper){...});

or when a new message has been created:

DevToolsSDK.webconsole.onLogEvent(function(aLogNode){...});

etc...
Attachment #514551 - Flags: feedback?
Attachment #514551 - Flags: feedback? → feedback?(rcampbell)
Attached patch v 0.2 removed log message (obsolete) — Splinter Review
whoops. killed log() call
Attachment #514551 - Attachment is obsolete: true
Attachment #514551 - Flags: feedback?(rcampbell)
Comment on attachment 514551 [details] [diff] [review]
v 0.1added several notifications

per irc...

+    let id = new String(this.hudId);
+    log(this.hudId + "\n\n");

lose the log.

+                      nodeId: aNode.getAttribute("id") };
+    notifyObj.wrappedJSObject = notifyObj;

lose that line.
Attachment #514551 - Flags: feedback+
Attachment #514557 - Attachment is obsolete: true
Attachment #514561 - Flags: review?(dtownsend)
Attachment #514561 - Flags: review?(dtownsend)
Attached patch v 0.4 - its only 4.6k! (obsolete) — Splinter Review
Removed console modified observer notification due to trouble testing it - tests included, all pass.
Attachment #514561 - Attachment is obsolete: true
Attachment #515170 - Flags: review?(dtownsend)
Comment on attachment 515170 [details] [diff] [review]
v 0.4 - its only 4.6k!

Drivers: I know there is no review yet... This is a tiny patch that will give the forthcoming DevTools SDK and the Addons SDK a way to know as soon as a console is created, destroyed or a message is logged to it. Addon authors (and the devtools team using the Addons SDK) will be able to easily extend the console and the console UI. Would love approval!
Attachment #515170 - Flags: approval2.0?
added test to patch.
Attachment #515170 - Attachment is obsolete: true
Attachment #515170 - Flags: review?(dtownsend)
Attachment #515170 - Flags: approval2.0?
Attachment #515176 - Flags: review?(dtownsend)
Attachment #515176 - Flags: approval2.0?
Comment on attachment 515176 [details] [diff] [review]
v 0.4.1 only 8.6 k, tiny!

Might be worth verifying that the node ID passed with the message creation is something useful.
Attachment #515176 - Flags: review?(dtownsend)
Attachment #515176 - Flags: review+
Attachment #515176 - Flags: approval2.0?
Attachment #515176 - Flags: approval2.0+
Depends on: 637401
http://hg.mozilla.org/mozilla-central/rev/e2a82bfadc67
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0
After several hours of hacking on some SDK stuff for the console, I realized that this notification is sent just milliseconds before the HeadsUpDisplay object is registered with the HUDService. I do after all need to get a reference to the HeadsUpDisplay at the moment the notification comes in.

patch forthcoming
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #516151 - Flags: review?(dtownsend)
Attachment #516151 - Flags: review?(dtownsend)
Attachment #516151 - Flags: review+
Attachment #516151 - Flags: approval2.0+
http://hg.mozilla.org/mozilla-central/rev/3e456bd488af
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Comment on attachment 516448 [details] [diff] [review]
v. 0.1 experimental for talos

Pushed to try server, ran talos tests, all tests passed: http://tbpl.mozilla.org/?tree=MozillaTry&rev=e30fab3e4af7

I think this was a fluke. So strange.
Attachment #516448 - Flags: review?(dtownsend)
Attachment #516448 - Flags: approval2.0?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
there were a number of other talos regressions yesterday as well. Not pointing at this patch.

from dev.tree-management

Regression :( Ts, MAX Dirty Profile increase 14.5% on MacOSX 10.5.8 Firefox
---------------------------------------------------------------------------
   Previous: avg 709.790 stddev 19.505 of 30 runs up to revision b6fd106a4eb3
   New     : avg 812.432 stddev 21.975 of 5 runs since revision 4b7c9efbf269
   Change  : +102.642 (14.5% / z=5.262)
   Graph   : http://mzl.la/gBOaPc

Changeset range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b6fd106a4eb3&tochange=4b7c9efbf269

Changesets:
 * http://hg.mozilla.org/mozilla-central/rev/290403971149
   : Marco Bonardo <mbonardo@mozilla.com> - Bug 637957 - Make results notifier hold a strong reference to  mCallback, to prevent it being destroyed prematurely.
r=sdwilsh a=blocker
   : http://bugzilla.mozilla.org/show_bug.cgi?id=637957

 * http://hg.mozilla.org/mozilla-central/rev/7bc88a8f9095
   : Bjarne Herland <bjarne@runitsoft.com> - Bug 634906: Allow Allow-Origin:null in CORS. r/a=sicking
   : http://bugzilla.mozilla.org/show_bug.cgi?id=634906

 * http://hg.mozilla.org/mozilla-central/rev/4b7c9efbf269
   : Boris Zbarsky <bzbarsky@mit.edu> - Bug 637644.  Start layout on the new document in CreateAboutBlankContentViewer, since if we don't do it here no one ever will.  r=jst, a=blocker
   : http://bugzilla.mozilla.org/show_bug.cgi?id=637644

Bugs:
 * http://bugzilla.mozilla.org/show_bug.cgi?id=634906
 * http://bugzilla.mozilla.org/show_bug.cgi?id=637644
 * http://bugzilla.mozilla.org/show_bug.cgi?id=637957
Comment on attachment 516448 [details] [diff] [review]
v. 0.1 experimental for talos

too late for Firefox 4. sorry. (also, we generally don't approve unreviewed patches)
Attachment #516448 - Flags: approval2.0? → approval2.0-
(In reply to comment #18)
> Comment on attachment 516448 [details] [diff] [review]
> v. 0.1 experimental for talos
> 
> too late for Firefox 4. sorry. (also, we generally don't approve unreviewed
> patches)

Did you look at the patch? Its moving 2 lines to another method due to talos apparent flakiness.
Whiteboard: [approved-patches-landed]
Comment on attachment 516448 [details] [diff] [review]
v. 0.1 experimental for talos

Looks like talos noise as the spike has gone away. also the try server test with this patch was not spiking either.
Attachment #516448 - Flags: review?(dtownsend)
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.