[DevTools] Customized devtools css doesn't work with the third party themes from Nightly 44.0a1

RESOLVED FIXED in Firefox 45

Status

()

Firefox
Developer Tools
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: magicp, Unassigned)

Tracking

({addon-compat})

44 Branch
Firefox 45
addon-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151001030236

Steps to reproduce:

1. Open Nightly 44.0a1 with third party theme (e.g. Qute 5++)
2. Open Developer Tools


Actual results:

Devtools css folder structure was changed from Nightly 44.0a1. So Nightly does not refer the third party themes' css now.


Expected results:

Does this change say "don't modify developer tools css !" ? If so, please fix devtools UI issues more quickly. Especially RTL locales issues. Because some issues have been solved by tentative solution of the third party themes.
(Reporter)

Updated

3 years ago
Component: Untriaged → Developer Tools
OS: Unspecified → All
Hardware: Unspecified → All
The themes would need to be updated.  All DevTools files have had path changes.  The same is true of DevTools add-ons, many of them have also needed updates.

At the moment, the DevTools themes are under chrome://devtools/skin/themes/*.

There is some further discussion of tweaking themes in bug 1207976, but it's unlikely the paths will go entirely back to what they were, so still themes would need updates either way.

My impression so far has been that themes are free to do what they want to style DevTools, but we're not attempting to provide any compatibility for them when things change.  So from time to time, they will need updates if we make breaking changes to DevTools files.  Brian, do you agree?
Flags: needinfo?(bgrinstead)
I am still not clear exactly how many addons are changing devtools css (intentionally or not), and the level of breakage that happened with the file move.  Specifically:

1) Does this affect *every* full theme, or only ones that have somehow opted into styling devtools
2) What is the failure mode?  The custom styling isn't applied, the browser throws error pages when trying to load, somewhere in between?

We have pretty done some pretty major markup / css refactoring (not counting this latest file move) that would have been hard for addons to keep up with.  When doing this, we've been operating under the assumption that custom themes weren't trying to style devtools (besides inheriting changes from toolbarbuttons and the like).  It'd be nice to know if that assumption is wrong.
Flags: needinfo?(bgrinstead) → needinfo?(magicp.jp)
See Also: → bug 1207976
(In reply to magicp from comment #0)
> Does this change say "don't modify developer tools css !" ? If so, please
> fix devtools UI issues more quickly. Especially RTL locales issues. Because
> some issues have been solved by tentative solution of the third party themes.

Can you please point me to any RTL bugs / other theme bugs on file so we can track that work properly?
(Reporter)

Comment 4

3 years ago
(In reply to Brian Grinstead [:bgrins] from comment #3)
> (In reply to magicp from comment #0)
> > Does this change say "don't modify developer tools css !" ? If so, please
> > fix devtools UI issues more quickly. Especially RTL locales issues. Because
> > some issues have been solved by tentative solution of the third party themes.
> 
> Can you please point me to any RTL bugs / other theme bugs on file so we can
> track that work properly?

You can easy to see the theme issues using LavaFox V2. My themes Qute 5++/6++ were replaced all default theme style by the latest Nightly.

Please find the following search URL.

https://bugzilla.mozilla.org/buglist.cgi?f1=OP&emailreporter1=1&o3=equals&list_id=12587291&short_desc=RTL&f0=OP&v3=magicp.jp%40gmail.com&emailtype1=exact&o2=notequals&f4=CP&query_format=advanced&emailassigned_to1=1&j1=OR&f3=reporter&f2=bug_status&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&short_desc_type=allwordssubstr&f5=CP&email1=magicp.jp%40gmail.com&v2=UNCONFIRMED
Flags: needinfo?(magicp.jp)
(In reply to magicp from comment #4)
> (In reply to Brian Grinstead [:bgrins] from comment #3)
> > (In reply to magicp from comment #0)
> > > Does this change say "don't modify developer tools css !" ? If so, please
> > > fix devtools UI issues more quickly. Especially RTL locales issues. Because
> > > some issues have been solved by tentative solution of the third party themes.
> > 
> > Can you please point me to any RTL bugs / other theme bugs on file so we can
> > track that work properly?
> 
> You can easy to see the theme issues using LavaFox V2. My themes Qute
> 5++/6++ were replaced all default theme style by the latest Nightly.

I'll take that to mean the failure mode is that custom stylesheets aren't applied.  I'm still not clear if this is happening on every theme or just ones that chose to make modifications to devtools
Created attachment 8669211 [details]
lavafox-43.png

screenshot of toolbox on 43 with lavafox theme applied
Created attachment 8669213 [details]
lavafox-44.png

screenshot of toolbox on 44 with lavafox theme applied
Lavafox in particular has some obvious regressions in 44 with regard to the inspector breadcrumbs and white tab text, although it does have many errors on 43 as well.  Oddly enough, the inspector sidebar panels look better on 44, which might suggest that some of the overrides were outdated and causing issues like the pseudo elements button to not show up.
Created attachment 8669215 [details]
qute-43.png

screenshot of toolbox on 43 with Qute 6++ theme applied
Created attachment 8669216 [details]
qute-44.png

screenshot of toolbox on 44 with Qute 6++ theme applied
(Reporter)

Comment 11

3 years ago
See Also:
Bug 1178102
Bug 1199126
Bug 1184019
Bug 1210641
Qute 6++ is clearly supporting the toolbox much more actively in 43 than the example from Comment 6.  Note: One thing I'm taking away looking at the screenshot from 43 is that it might help to have an easy way for a theme to provide overrides for our CSS variables and/or have a way to load a single CSS file inside of all of the frames after the normal theming where the author could put whatever you wanted.
(In reply to magicp from comment #11)
> See Also:
> Bug 1178102
> Bug 1199126
> Bug 1184019
> Bug 1210641

So, if you have fixes for these and other issues in your themes, then ideally we should be pushing these into m-c so you don't need to maintain a fork of the styles to fix these bugs, and everyone would benefit from the fixes.
(Reporter)

Comment 14

3 years ago
Thank you in advance.

Comment 15

3 years ago
Also my themes are impacted by this change: LittleFox, Nautipolis, Walnut, Walnut2, Bricks, Metal and Microfox.

Comment 16

3 years ago
Created attachment 8669402 [details]
chrome.manifest

This is an example on how to redirect the new .css references to the existing ones in browser/skin/devtools. One could also do the other way around.
(Reporter)

Updated

3 years ago
Status: UNCONFIRMED → NEW
status-firefox44: --- → affected
Ever confirmed: true
The file migration has had breaking changes for both themes and add-ons, but we're hopeful the simpler file structure will help us all improve DevTools more efficiently going forward.

Just like with DevTools add-ons, any themes that patch DevTools files will need to update for these new paths.  No one's trying to say "don't patch DevTools files".  We always reserve the right to move or change any files as needed, however.

It is possible for theme authors to override the new URLs without duplicating images, such as this example: https://bug1210652.bmoattachments.org/attachment.cgi?id=8669402

As the DevTools file migration makes its way to release channel, these path overrides will no longer be needed, since the themes can instead override only the new paths.

As :bgrins said in comment 11, issues with themes, RTL, etc. should be track by their own individual bugs.  If you already have fixes, please attach patches to the appropriate bugs so that everyone can benefit.

I also created an RTL meta-bug 1216762 based on the search in comment 4.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox44: affected → ---
Keywords: addon-compat
Resolution: --- → WONTFIX
(Reporter)

Comment 18

3 years ago
I understand. Thanks a lot.
We decided to reopen bug 1207976 and remove "/themes" from the new URLs.

Theme authors will still need to make changes, but the changes are less extensive now, as you should not need to override each individual file.
Status: RESOLVED → REOPENED
Depends on: 1207976
Resolution: WONTFIX → ---
See Also: bug 1207976
Given the feedback in bug 1207976, this situation should now be... "better", in that less changes are needed by theme authors, so marking FIXED since I believe it's about as good as we're likely to make it.  Changes *are* still needed for themes that style DevTools, so they still won't magically work without change.

See bug 1207976 comment 29 for an example of the changes needed.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
You need to log in before you can comment on or make changes to this bug.