696755 - automatically open the inspector sidebar on inspect if previously-active

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Luke Crouch [:groovecoder]]

When I inspect an element, I want it to automatically open to the HTML panel.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

The best solution is probably to remember the open / closed state of all registered tools.

Comment 2

[Luke Crouch [:groovecoder]]

Totally agree. I want to hit a key combination (cmd+shift+i is fine) and switch to "development mode" - open the console, inspector, and/or scratchpad depending if I had them open last time.

Comment 3

[Rob Campbell [:rc] (:robcee)]

I'm not sure this will be true for everyone.

The Web Console and Scratchpad are currently independent of the other tools. This will change somewhat in the near future, however and it will make sense to show them all when we have the global toolbar. (bug needed)

For the highlighter specifically, I'm not sure everyone will always want the HTML panel open or the Style Inspector or (insert tool here). They're intended as task-specific tools. With the addition of Breadcrumbs (bug 672006), I don't think the HTML panel will be needed for all tree navigation.

Anyway, it's a good bug to have. We should probably start a discussion on dev.apps.firefox to try to figure this out.

Comment 4

[Kevin Dangoor]

The breadcrumbs will certainly change the dynamic here. The addition of the developer toolbar may change the dynamic again.

Rob is right that the general idea here is to open the tools you need for the task at hand. It *may* be the case that people will want the HTML panel to always be displayed when they open the highlighter, but it would be good to see what people think of the breadcrumbs first.

Comment 5

[Paul Rouget [:paul]]

We should make the state of this panel persistent. We should do that in bug 689946.

Comment 6

[Rob Campbell [:rc] (:robcee)]

I'd rather not conflate that bug with this one. This is about opening the panels, the HTML size bug should be about persisting the height of that and maybe the width of the sidebar when it lands.

Comment 8

[Heather Arthur [:harth]]

Attachment: Reduced patch demonstrating Style Inspector initialization problem

Here's a WIP. Whenever the highlighter is "restored" (on first open, switching back after being on another tab), the tools are reopened; this just hard codes a set of tools to open on first load. It works for the Rule View and HTML panel, but gets an error for the Style Inspector:

Error: element is null
Source File: resource:///modules/devtools/CssLogic.jsm
Line: 689

Only when using the context menu "Inspect Element" because that doesn't give the Style Inspector enough time to initialize before inspecting a particular element.

Waiting for the "StyleInspector-opened" event fixes this problem, but we need more general initialization handling if this is something that could affect other highlighter-based tools.

Comment 9

[Paul Rouget [:paul]]

(In reply to Heather Arthur [:harth] from comment #8)
> Waiting for the "StyleInspector-opened" event fixes this problem, but we
> need more general initialization handling if this is something that could
> affect other highlighter-based tools.

Can you give a bit more details about this problem? Maybe open a new bug.

Comment 10

[Heather Arthur [:harth]]

(In reply to Paul Rouget [:paul] from comment #9)
> (In reply to Heather Arthur [:harth] from comment #8)
> > Waiting for the "StyleInspector-opened" event fixes this problem, but we
> > need more general initialization handling if this is something that could
> > affect other highlighter-based tools.
> 
> Can you give a bit more details about this problem? Maybe open a new bug.

Dcamp looked into this and figured out the problem. He's going to rewrite some of this code to make the Properties view initialization better.

Comment 11

[Dave Camp (:dcamp)]

(In reply to Heather Arthur [:harth] from comment #10)

> Dcamp looked into this and figured out the problem. He's going to rewrite
> some of this code to make the Properties view initialization better.

To elaborate a bit, registered tools provide an isOpen() method, which for style sidebar tools is implemented as "(My iframe is loaded and initialized) && (I am the currently-active sidebar tool, and the sidebar is showing)"

The style inspector protects itself from being updated while !isOpen by queuing the node to be highlighted until it has been opened.  If that were working, this bug wouldn't happen.  But the highlighter itself doesn't even bother to update the panels if they aren't open.  So the queuing doesn't happen.

isOpen conflating "active panel" and "loaded and initialized" makes this all more confusing than it needs to be.  I started to separate that out a bit.

Comment 12

[Paul Rouget [:paul]]

> It works for the Rule View and HTML panel, but gets an error for the Style Inspector.
Can we get a first patch that implements only the HTML panel persistence?

Comment 16

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Bug triage, filter on PEGASUS.

Comment 17

[Anthony Ricaud (:rik)]

Should we open another bug for Style Inspector? This is the panel that I'm most interested to see opened by default.

Comment 18

[Paul Rouget [:paul]]

If I understand correctly:
- persistence of the HTML Tree state is easy.
- persistence of the Sidebar state is hard.

The Sidebar problem should not block the HTML Tree persistence.
I am opening a different bug just for the Style sidebar: bug 721745

Comment 19

[Paul Rouget [:paul]]

taking this bug

Comment 20

[Paul Rouget [:paul]]

Sounds like this is going to be fixed in bug 719607.

Comment 21

[Dave Camp (:dcamp)]

It will be fixed for the HTML panel in bug 719607, but not the sidebar.

Comment 22

[Dave Camp (:dcamp)]

Attachment: fix v1

Pending a larger reworking of registerTools, this will at least let us save the state of the sidebar for now.  Haven't written a test yet, but in case you're curious about the approach while I write that test...

Comment 23

[Dave Camp (:dcamp)]

Attachment: fix v2

Now with a test.

Comment 24

[Dave Camp (:dcamp)]

I removed saveToolState and removeToolState in favor of sidebar-specific saving.  This will break registerTools that aren't sidebars.  But with bug 719607 we don't have any tools that aren't sidebars, and we're planning on messing more with  registerTools in the very near future anyway.

Comment 25

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 592443 [details] [diff] [review]
fix v2

I have not had a chance to test this, but codewise this looks excellent.

Comment 26

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/31daf7fba165

Comment 27

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/31daf7fba165