Closed Bug 1127234 Opened 5 years ago Closed 5 years ago

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

Categories

(Toolkit :: Themes, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: Margaret, Assigned: canova, Mentored)

References

Details

(Whiteboard: [lang=css][good first bug])

Attachments

(2 files, 1 obsolete file)

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.
Whiteboard: [lang=css] → [lang=css][good first bug]
Attached patch Patch (obsolete) — Splinter Review
moved the necessary files to /themes/shared/ 
[i have not created a separate directory for the moved files]
Flags: needinfo?(ntim007)
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.
Flags: needinfo?(ntim007)
Assignee: nobody → 526avijitgupta
Status: NEW → ASSIGNED
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.
Attachment #8557803 - Flags: feedback-
Comment 11? lolno. Comment 2 is what I meant.
Avijit, are you still working on this?
Flags: needinfo?(526avijitgupta)
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..
Flags: needinfo?(526avijitgupta)
Assignee: 526avijitgupta → nobody
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.
Flags: needinfo?(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.
Flags: needinfo?(margaret.leibovic)
Moved files to /themes/shared and changed the windows and osx jar.mn files. I couldn't find their equivalent files in linux.
Attachment #8589599 - Flags: review?(jaws)
Assignee: nobody → canaltinova
Attachment #8557803 - Attachment is obsolete: true
Attachment #8589599 - Flags: review?(jaws) → review+
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.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/eabdece7fd76
Keywords: checkin-needed
Whiteboard: [lang=css][good first bug] → [lang=css][good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/eabdece7fd76
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=css][good first bug][fixed-in-fx-team] → [lang=css][good first bug]
Target Milestone: --- → mozilla40
Margaret and Jared, thanks for your helps!
jaws, can we request uplift for this? We need to uplift my patches for bug 1154028, which are based on top of this.
Flags: needinfo?(jaws)
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
Flags: needinfo?(jaws)
Attachment #8589599 - Flags: approval-mozilla-beta?
Attachment #8589599 - Flags: approval-mozilla-aurora?
Comment on attachment 8589599 [details] [diff] [review]
bug1127234_movedtosharedfolder.diff

Too late for 38 but we can take a fix for 38.0.5.
Attachment #8589599 - Flags: approval-mozilla-beta?
Attachment #8589599 - Flags: approval-mozilla-beta+
Attachment #8589599 - Flags: approval-mozilla-aurora?
Attachment #8589599 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.