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

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[magicp]

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.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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?

Comment 2

[Brian Grinstead [:bgrins]]

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.

Comment 3

[Brian Grinstead [:bgrins]]

(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?

Comment 4

[magicp]

(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

Comment 5

[Brian Grinstead [:bgrins]]

(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

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: lavafox-43.png

screenshot of toolbox on 43 with lavafox theme applied

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: lavafox-44.png

screenshot of toolbox on 44 with lavafox theme applied

Comment 8

[Brian Grinstead [:bgrins]]

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.

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: qute-43.png

screenshot of toolbox on 43 with Qute 6++ theme applied

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: qute-44.png

screenshot of toolbox on 44 with Qute 6++ theme applied

Comment 11

[magicp]

See Also:
Bug 1178102
Bug 1199126
Bug 1184019
Bug 1210641

Comment 12

[Brian Grinstead [:bgrins]]

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.

Comment 13

[Brian Grinstead [:bgrins]]

(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.

Comment 14

[magicp]

Thank you in advance.

Comment 15

[Alfred Kayser]

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

Comment 16

[Alfred Kayser]

Attachment: 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.

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 18

[magicp]

I understand. Thanks a lot.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 20

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.