Closed Bug 1388731 Opened 2 years ago Closed 2 years ago
Use a shared jar
.mn in themes to simplify changes
I'm planning to split the combined toolbar SVG into separated SVGs like FX has done also for performance reasons. With our actual jar.mn files we have to add/change the shared files in every platform jar.mn for it's own. It would be simpler if we could do this in a shared jar.mn and will be included in the platform ones.
Moved the shared files to shared/jar.inc.mn. I also moved the communicator and editor blocks to the end when they aren't already there to make the files a bit more consistent and cleaned up over the platforms. In a future bug we could also sort them alphabetically.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8895361 - Flags: review?(jorgk)
Tried the patch on all platforms and it looks all is working as it should.
I'm a little bit confused by this patch. You're mixing two things, the shared stuff, and moving stuff around. I don't want to be a pain, but that should really be two patches. As for the shared stuff: You create 32 lines in jar.inc.mn, 27 for all platforms, 1 for Linux/Windows, 4 for Mac. However, the three platforms don't have all those: Linux has 26, Windows 28 and Mac 31. So Linux is getting something it didn't have before, counted with grep -i shared linux\jar.mn | wc -l The other thing I find a little strange is that a file that supposedly contains "shared" things, has an #ifdef (or #ifndef) to exclude things which aren't shared after all. I'm willing to accept that, but I'd like you to check what's going on with Linux and why that had 26 shared things before and now it's getting 28.
The two added files are email@example.com and firstname.lastname@example.org. Until now Linux had almost no HiDPI support but now Gnome, KDE etc. have it now and I'm planning to use this files for this.
Comment on attachment 8895361 [details] [diff] [review] shared-jar.inc.mn.patch OK, I did the bean counting for the shared stuff, so I hope you got stuff right that you moved around ;-(
Attachment #8895361 - Flags: review?(jorgk) → review+
Oh, I forgot to say: Next time please make it easier for the reviewer and split unrelated things into multiple patches.
My local builds on all platform looked good. So I should have moved all correctly.
(In reply to Richard Marti (:Paenglab) from comment #7) > My local builds on all platform looked good. So I should have moved all > correctly. And you tried do insert an image and check the alignment icon? ;-) Anyway, I checked patch again and counted the lines in the moved blocks, all Swiss quality :-)
Pushed by email@example.com: https://hg.mozilla.org/comm-central/rev/7cdf95b6e5ad Unify the shared elements in a shared jar.inc.mn. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.