The HTML Tree Panel should NOT use the registerTools mechanism

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: paul, Unassigned)

Tracking

Trunk
Firefox 13
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

6 years ago
The HTML Tree is not a Node Centric tool but just a feature of the Inspector. Also, since it uses the registerTools API, we can't move the <HTML> button where ever we want.

See also bug 717924
(Reporter)

Comment 1

6 years ago
Created attachment 590411 [details] [diff] [review]
patch v1
(Reporter)

Updated

6 years ago
No longer depends on: 707809
(Reporter)

Updated

6 years ago
Attachment #590411 - Flags: review?(rcampbell)
(Reporter)

Updated

6 years ago
Depends on: 717924
(Reporter)

Updated

6 years ago
Blocks: 696755
(Reporter)

Comment 2

6 years ago
Created attachment 592140 [details] [diff] [review]
Patch v2
(Reporter)

Updated

6 years ago
Attachment #590411 - Attachment is obsolete: true
Attachment #590411 - Flags: review?(rcampbell)
(Reporter)

Comment 3

6 years ago
I forgot to save the Tree state if the inspector get "hidden". And I actually added a preference to save the latest state of the Tree (and that's gonna fix bug 696755).
(Reporter)

Updated

6 years ago
Attachment #592140 - Flags: review?(rcampbell)
Comment on attachment 592140 [details] [diff] [review]
Patch v2

This looks good to me. Thanks!
Attachment #592140 - Flags: review?(rcampbell) → review+
Whiteboard: [land-in-fx-team]
Whiteboard: [land-in-fx-team] → [waiting-on-blocking-bug]
Blocks: 584407

Comment 5

6 years ago
Created attachment 592442 [details] [diff] [review]
addendum

To be applied on top of patch v2:

* Fixes tests.
* Fixes another conflated "opened" and "loaded" mess.  The current patch exacerbates an existing problem (with this patch, try opening the html panel and then toggling the inspector on and off really quickly with the keybinding, and watch a stack of splitters build up).
* Gets rid of !openInDock. That codepath was never used, and made fixing the previous bullet point more difficult.
Attachment #592442 - Flags: review?(rcampbell)
Comment on attachment 592442 [details] [diff] [review]
addendum

clean.
Attachment #592442 - Flags: review?(rcampbell) → review+
Comment on attachment 592442 [details] [diff] [review]
addendum

>--- a/browser/devtools/highlighter/test/head.js
>+++ b/browser/devtools/highlighter/test/head.js
>@@ -34,17 +34,26 @@
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> const Cu = Components.utils;
> let tempScope = {};
> Cu.import("resource:///modules/devtools/LayoutHelpers.jsm", tempScope);
>+Cu.import("resource://gre/modules/Services.jsm", tempScope);
> let LayoutHelpers = tempScope.LayoutHelpers;
>+let Services = tempScope.Services;

Services is already defined here.

>+// Clear preferences that may be set during the course of tests.
>+function clearUserPrefs()
>+{
>+  Services.prefs.clearUserPref("devtools.inspector.htmlPanelOpen");
>+}
>+clearUserPrefs();

I don't know why exactly this is needed, but you generally need to clean up after tests rather than before them. I.e.:

registerCleanupFunction(function () {
  Services.prefs.clearUserPref("devtools.inspector.htmlPanelOpen");
});

Comment 8

6 years ago
Created attachment 592739 [details] [diff] [review]
addendum v2
Attachment #592442 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Blocks: 724507
Blocks: 722552

Updated

6 years ago
Blocks: 726518
I think these can land. I'm going to send them through try server today to make sure.
Status: NEW → ASSIGNED
Created attachment 599077 [details] [diff] [review]
Patch v2 - rebased
Attachment #592140 - Attachment is obsolete: true
Created attachment 599078 [details] [diff] [review]
addendum v2 - rebased
Attachment #592739 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Whiteboard: [waiting-on-blocking-bug] → [land-in-fx-team]
(Reporter)

Comment 12

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/c573765690ce
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c573765690ce
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
You need to log in before you can comment on or make changes to this bug.