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

RESOLVED FIXED in Firefox 38.0.5

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Margaret, Assigned: canaltinova, Mentored)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 wontfix, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed)

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

4 years ago
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.

Updated

4 years ago
Whiteboard: [lang=css] → [lang=css][good first bug]

Comment 1

4 years ago
Posted 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 2

4 years ago
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.

Updated

4 years ago
Flags: needinfo?(ntim007)

Updated

4 years ago
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)

Comment 6

4 years ago
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)

Updated

4 years ago
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)
Reporter

Comment 8

4 years ago
(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)

Updated

4 years ago
Assignee: nobody → canaltinova

Updated

4 years ago
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: 4 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!
Reporter

Comment 14

4 years ago
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.