Closed
Bug 882761
Opened 11 years ago
Closed 10 years ago
Package all theme files in a unified XPI
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
3.1
People
(Reporter: Paenglab, Assigned: Paenglab)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
92.53 KB,
patch
|
Paenglab
:
review+
|
Details | Diff | Splinter Review |
With ical.js where will be no need to package platform specific XPIs. But this needs all platform theme files in one XPI.
Assignee | ||
Comment 1•11 years ago
|
||
This patch adds in chrome folder a skin folder. In this folder there there this stucture: - common (the common theme files) - linux (the Linux specific files) - osx (the OSX specific files) - windows (the Windows specific files) 'skin calendar' and 'skin lightning' will be registered platform depending to the calendar and lightning folder in the platform folders. To access the common files, chrome://calendar-common/skin/... will do this. The problem I'm seeing is, the manifest file isn't created with the same order as the entries are in jar.mn. This makes it impossible to add other platforms to register to a default theme folder. The order need to be strictly like this to work on all platforms correctly: % skin calendar classic/1.0 chrome/skin/linux/calendar/ % skin calendar classic/1.0 chrome/skin/osx/calendar/ os=Darwin % skin calendar classic/1.0 chrome/skin/windows/calendar/ os=WINNT This would mean, linux is the default folder for all platforms except OS X and Windows. This two have their own folders. Linux is only a proposal as I think, linux is not optimal because GTX-icons don't be supported on all other platforms. If somebody knows how to fix the order in manifest, please tell how to do.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
I'm waiting after the merge to TB 25 Lightning 2.7 for review to not risk breakage in the ESR release.
Comment 3•11 years ago
|
||
I think Linux being the default is our best bet. Aside from OS/2, there are likely a few other unix flavors that will not be covered by using "Linux". I'm often angry with chrome registration flags not being flexible enough. Afaik we can't just do this: > skin calendar classic/1.0 chrome/skin/linux/calendar/ os!=Darwin os!=WINNT So here is a hack that might work. In the main file: > include notwindows.manifest os!=WINNT Then in notwindows.manifest: > include general.manifest os!=Darwin Then in general.manifest > skin calendar classic/1.0 chrome/skin/linux/calendar/ Filenames are subject to change of course.
Assignee | ||
Comment 4•11 years ago
|
||
Or instead of generating the manifest file, create it with the correct order and copy it to the right place with jar.mn. Would this work? I think it's no difference when new or changed rules are written in manifest file than in jar.mn.
Comment 5•11 years ago
|
||
This means we need one central file for the manifest, I think thats counterproductive. Maybe instead we could fix the manifest parser to retain the local order of lines per file.
Assignee | ||
Comment 6•11 years ago
|
||
I filed bug 891476 for a fixed order during build.
Depends on: 891476
Assignee | ||
Comment 7•11 years ago
|
||
Okay, let's try a review to have enough time until 31ESR. I'm using it on every build and saw no issue on Windows and Linux.
Attachment #762130 -
Attachment is obsolete: true
Attachment #8333985 -
Flags: review?(philipp)
Comment 8•10 years ago
|
||
Comment on attachment 8333985 [details] [diff] [review] patch Review of attachment 8333985 [details] [diff] [review]: ----------------------------------------------------------------- r+ with these comments considered/clarified. ::: calendar/base/jar.mn @@ +113,5 @@ > % style chrome://global/content/customizeToolbar.xul chrome://calendar/skin/calendar-task-view.css > % style chrome://global/content/customizeToolbar.xul chrome://calendar/skin/calendar-event-dialog.css > +% style chrome://calendar/content/calendar-event-dialog.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css > +% style chrome://calendar/content/calendar-event-dialog-attendees.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css > + ../skin/common/alarm-flashing.png (themes/common/images/alarm-flashing.png) Just for clarification: using the parent directory here does what to the final path? @@ +226,5 @@ > % style chrome://calendar/content/calendar-event-dialog.xul chrome://calendar-windows/skin/calendar-event-dialog.css os=WINNT > % style chrome://calendar/content/calendar-event-dialog-attendees.xul chrome://calendar-windows/skin/calendar-event-dialog.css os=WINNT > % style chrome://calendar/content/calendar-occurrence-prompt.xul chrome://calendar-windows/skin/calendar.css os=WINNT osversion>=6 > +% skin calendar-windows classic/1.0 chrome/skin/windows/calendar/win-classic/ os=WINNT osversion<6 > +% skin calendar-windows classic/1.0 chrome/skin/windows/calendar/win-aero/ os=WINNT osversion>=6 Do you think it makes sense to move these skin registrations up to the place where the other skin registrations for Linux/Mac/Windows are done? ::: calendar/lightning/themes/linux/imip.css @@ +1,5 @@ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > +@import url(chrome://calendar-common/skin/imip.css); Shouldn't this be lightning-common (also in a few other places) ? Even though we only have one product in tree now, I don't think we should mix calendar/lightning just yet.
Attachment #8333985 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #8) > Comment on attachment 8333985 [details] [diff] [review] > patch > > Review of attachment 8333985 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ with these comments considered/clarified. > > ::: calendar/base/jar.mn > @@ +113,5 @@ > > % style chrome://global/content/customizeToolbar.xul chrome://calendar/skin/calendar-task-view.css > > % style chrome://global/content/customizeToolbar.xul chrome://calendar/skin/calendar-event-dialog.css > > +% style chrome://calendar/content/calendar-event-dialog.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css > > +% style chrome://calendar/content/calendar-event-dialog-attendees.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css > > + ../skin/common/alarm-flashing.png (themes/common/images/alarm-flashing.png) > > Just for clarification: using the parent directory here does what to the > final path? This is to go out of the calendar and lightning directory and to create a skin directory at the same level of calendar and lightning. > @@ +226,5 @@ > > % style chrome://calendar/content/calendar-event-dialog.xul chrome://calendar-windows/skin/calendar-event-dialog.css os=WINNT > > % style chrome://calendar/content/calendar-event-dialog-attendees.xul chrome://calendar-windows/skin/calendar-event-dialog.css os=WINNT > > % style chrome://calendar/content/calendar-occurrence-prompt.xul chrome://calendar-windows/skin/calendar.css os=WINNT osversion>=6 > > +% skin calendar-windows classic/1.0 chrome/skin/windows/calendar/win-classic/ os=WINNT osversion<6 > > +% skin calendar-windows classic/1.0 chrome/skin/windows/calendar/win-aero/ os=WINNT osversion>=6 > > Do you think it makes sense to move these skin registrations up to the place > where the other skin registrations for Linux/Mac/Windows are done? It works also at this position, but I moved then in my last patch to the other skin registrations. Then all are together. > ::: calendar/lightning/themes/linux/imip.css > @@ +1,5 @@ > > /* This Source Code Form is subject to the terms of the Mozilla Public > > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > > > +@import url(chrome://calendar-common/skin/imip.css); > > Shouldn't this be lightning-common (also in a few other places) ? Even > though we only have one product in tree now, I don't think we should mix > calendar/lightning just yet. I've created now a lightning-common. There are now a common and a lightning-common directory in skin. I don't rename common to calendar-common for when we merge both together, okay? I made a try run to check nothing breaks: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=57458737a743 The Try-build Lightning xpis can be found here: http://ftp.mozilla.org/pub/mozilla.org/calendar/lightning/try-builds/richard.marti@gmail.com-57458737a743/
Attachment #8333985 -
Attachment is obsolete: true
Attachment #8357324 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
Check-in: http://hg.mozilla.org/comm-central/rev/b55d96af36a4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.1
Comment 11•10 years ago
|
||
Fwiw on OpenBSD it seems this broke lightning display as noted in comment 3, tried with seamonkey 2.26b1 with bundled lightning 3.1. I'll file a followup to see how i can make non-tier1-unix-oses-that-are-not-mac use the linux skin.
You need to log in
before you can comment on or make changes to this bug.
Description
•