821251 - Remove devtools-browser.css from browser.xul

Status: NEW

(DevTools :: General, defect, P3)

Description

[Paul Rouget [:paul]]

In theory, this file won't be needed once bug 818033 is fixed.

Comment 1

[Paul Rouget [:paul]]

It is used by the responsive design mode.

Comment 2

[Jesal]

Hi Paul, I am interested in this bug, is this fix still needed? could you explain why we need to remove this file?

Thanks 

Jesal

Comment 3

[Paul Rouget [:paul]]

(In reply to Jesal from comment #2)
> Hi Paul, I am interested in this bug, is this fix still needed? could you
> explain why we need to remove this file?

Hi Jesal.

You can see in browser.xul (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul) that we include chrome://browser/skin/devtools/common.css. I believe this is not useful anymore. This needs to be verified, and then just removing the line should be enough.

The only devtools that are implemented directly in browser.xul are:
- developer toolbar
- eyedropper
- responsive mode

So we need to make sure these tools are not affected.

Comment 4

[Jesal]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #3)
> (In reply to Jesal from comment #2)
> > Hi Paul, I am interested in this bug, is this fix still needed? could you
> > explain why we need to remove this file?
> 
> Hi Jesal.
> 
> You can see in browser.xul
> (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> xul) that we include chrome://browser/skin/devtools/common.css. I believe
> this is not useful anymore. This needs to be verified, and then just
> removing the line should be enough.
> 
> The only devtools that are implemented directly in browser.xul are:
> - developer toolbar
> - eyedropper
> - responsive mode
> 
> So we need to make sure these tools are not affected.

Thanks for the response Paul. So I would I test if the other tools are not affected, is it just a case of removing that line and seeing if functionality of the other tools are the same? How would I check this in the browser?

Thanks

Jesal

Comment 5

[Paul Rouget [:paul]]

(In reply to Jesal from comment #4)
> (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from
> comment #3)
> > (In reply to Jesal from comment #2)
> > > Hi Paul, I am interested in this bug, is this fix still needed? could you
> > > explain why we need to remove this file?
> > 
> > Hi Jesal.
> > 
> > You can see in browser.xul
> > (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> > xul) that we include chrome://browser/skin/devtools/common.css. I believe
> > this is not useful anymore. This needs to be verified, and then just
> > removing the line should be enough.
> > 
> > The only devtools that are implemented directly in browser.xul are:
> > - developer toolbar
> > - eyedropper
> > - responsive mode
> > 
> > So we need to make sure these tools are not affected.
> 
> Thanks for the response Paul. So I would I test if the other tools are not
> affected, is it just a case of removing that line and seeing if
> functionality of the other tools are the same? How would I check this in the
> browser?

You need to remove the line and recompile.

I believe you'll see some issues with the splitters, sidebar size and eyedropper. You might have to split the file in 2 if it's the case.

Comment 6

[Jesal]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #5)
> (In reply to Jesal from comment #4)
> > (In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from
> > comment #3)
> > > (In reply to Jesal from comment #2)
> > > > Hi Paul, I am interested in this bug, is this fix still needed? could you
> > > > explain why we need to remove this file?
> > > 
> > > Hi Jesal.
> > > 
> > > You can see in browser.xul
> > > (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> > > xul) that we include chrome://browser/skin/devtools/common.css. I believe
> > > this is not useful anymore. This needs to be verified, and then just
> > > removing the line should be enough.
> > > 
> > > The only devtools that are implemented directly in browser.xul are:
> > > - developer toolbar
> > > - eyedropper
> > > - responsive mode
> > > 
> > > So we need to make sure these tools are not affected.
> > 
> > Thanks for the response Paul. So I would I test if the other tools are not
> > affected, is it just a case of removing that line and seeing if
> > functionality of the other tools are the same? How would I check this in the
> > browser?
> 
> You need to remove the line and recompile.
> 
> I believe you'll see some issues with the splitters, sidebar size and
> eyedropper. You might have to split the file in 2 if it's the case.

Hi Paul,
I have removed the line and recompiled. I see issues when the eyedropper tool, but I don't see any changes to the side bars or splitters, as I am looking in the developers tool bar, is there anywhere else I need to look?
Also How would I split the file, would I have to look at what depends on the eyedropper functions and change them?

Thanks

Jesal

Comment 7

[Paul Rouget [:paul]]

(In reply to Jesal from comment #6)
> I have removed the line and recompiled. I see issues when the eyedropper
> tool, but I don't see any changes to the side bars or splitters, as I am
> looking in the developers tool bar, is there anywhere else I need to look?

I don't think so. You might want to look at all the selectors in common.css
and see if any of these match something that runs in browser.xul.

> Also How would I split the file, would I have to look at what depends on the
> eyedropper functions and change them?

Remove from common.css all the things that are not useful anymore. See what's left.
If it's just the eyedropper, maybe we can inject the style when we create the pannel
(like panel.style.border="none",etc).

Comment 8

[Zac Svoboda :zacnomore]

browser.xul no longer exists, would that now be browser.xhtml?