1127234 - Store shared toolkit theme files in /themes/shared instead of /themes/windows

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[:Margaret Leibovic]

From bug 1117258 comment 9:

Can you please file a bug to move toolkit/themes/windows/global/aboutReader.css to toolkit/themes/shared ? We shouldn't be storing shared theme files like this in /windows. The bug can also cover about.css, aboutCache.css, aboutCacheEntry.css, aboutMemory.css, aboutSupport.css, and appPicker.css.

Comment 1

[Avijit Gupta]

Attachment: Patch

moved the necessary files to /themes/shared/ 
[i have not created a separate directory for the moved files]

Comment 2

[Tim Nguyen :ntim]

Comment on attachment 8557803 [details] [diff] [review]
Patch

Review of attachment 8557803 [details] [diff] [review]:
-----------------------------------------------------------------

Can you ask one of the mentors next time you need info or a review ? Thanks :)
Anyways, here are a couple of comments :

Can the changes be reflected as hg moves ?
hg move toolkit/themes/windows/filename toolkit/themes/shared/filename

Also, the jar.mn entries in toolkit/themes/*/ need to be updated to point to share.

One last thing, I think the osx and linux files can be removed now that their equivalents are in shared.

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 8557803 [details] [diff] [review]
Patch

Tim's remarks in comment 11 are a good place to start for getting this patch working and ready to land.

Comment 4

[Justin Dolske [:Dolske]]

Comment 11? lolno. Comment 2 is what I meant.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Avijit, are you still working on this?

Comment 6

[Avijit Gupta]

Yeah, I'm still interested to work on this.
I'm busy due to the ongoing university exams. I'll be back within a week or so..

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Hi, it seems Avijit stopped working on it. I'm new here. Can i start work on it? And if so, can i get some information about jar files. 
I find the css file paths but i don't exactly know how to change it to shared file. The file paths seems different than usual.

Comment 8

[:Margaret Leibovic]

(In reply to Nazim Can [:Canova] from comment #7)
> Hi, it seems Avijit stopped working on it. I'm new here. Can i start work on
> it? And if so, can i get some information about jar files. 
> I find the css file paths but i don't exactly know how to change it to
> shared file. The file paths seems different than usual.

Sure, you can take this!

However, I don't think I understand your question. Are you having trouble figuring out how to update the jar files? You should do a search for "shared" to see other places where we use shared theme files to see what the paths should look like.

Comment 9

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: bug1127234_movedtosharedfolder.diff

Moved files to /themes/shared and changed the windows and osx jar.mn files. I couldn't find their equivalent files in linux.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Toolkit themes in Linux are built on top of the Windows theme, that's why you couldn't find them for Linux. Nice job with the patch. I'll mark this checkin-needed now, and in a day or two it should make it's way to mozilla-central. The following day it will appear in Firefox Nightly.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eabdece7fd76

Comment 13

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Margaret and Jared, thanks for your helps!

Comment 14

[:Margaret Leibovic]

jaws, can we request uplift for this? We need to uplift my patches for bug 1154028, which are based on top of this.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8589599 [details] [diff] [review]
bug1127234_movedtosharedfolder.diff

Approval Request Comment
[Feature/regressing bug #]: reader mode styling changed places in the source tree
[User impact if declined]: none, but it will make developers work harder to port changes to aurora and beta as patches 
[Describe test coverage new/current, TreeHerder]: none, simple file moves
[Risks and why]: has potential to break 3rd party full themes that are changing the style of the about pages
[String/UUID change made/needed]: none

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8589599 [details] [diff] [review]
bug1127234_movedtosharedfolder.diff

Too late for 38 but we can take a fix for 38.0.5.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for Aurora 39 and Beta 38

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/releases/mozilla-beta/rev/25c1a831a2d4
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc8af0a8d3c2