Last Comment Bug 652306 - About dialog css shouldn't depend on MOZ_OFFICIAL_BRANDING
: About dialog css shouldn't depend on MOZ_OFFICIAL_BRANDING
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
-- normal (vote)
: Firefox 6
Assigned To: Mike Hommey [:glandium]
:
:
Mentors:
Depends on:
Blocks: 649366
  Show dependency treegraph
 
Reported: 2011-04-23 00:15 PDT by Mike Hommey [:glandium]
Modified: 2011-07-28 02:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposal (9.36 KB, patch)
2011-04-23 00:31 PDT, Mike Hommey [:glandium]
gavin.sharp: review+
Details | Diff | Splinter Review
Patch (9.29 KB, patch)
2011-04-23 01:19 PDT, Mike Hommey [:glandium]
mh+mozilla: review+
Details | Diff | Splinter Review

Description User image Mike Hommey [:glandium] 2011-04-23 00:15:42 PDT
The new about dialog for Nightly and Aurora has significant differences with the about dialog for Firefox. Bug 648362 made the main changes for this, and to do so added some ifdef MOZ_OFFICIAL_BRANDING in aboutDialog.css.

While this works well for Firefox, it doesn't work well for rebranding. Rebranded builds (obviously) don't set MOZ_OFFICIAL_BRANDING, yet, as they most likely inherit from the previous Firefox branding, they do use a logo image and not a background image, and have a clear background instead of dark.

I think aboutDialog.css should be split in a generic and a branding part.
Comment 1 User image Mike Hommey [:glandium] 2011-04-23 00:31:29 PDT
Created attachment 527919 [details] [diff] [review]
Proposal

Something like that (untested) could work. It would even allow brandings to override some more css and do some fancy stuff if they wanted to.
Comment 2 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-23 00:52:08 PDT
Comment on attachment 527919 [details] [diff] [review]
Proposal

>diff --git a/browser/branding/official/content/jar.mn b/browser/branding/official/content/jar.mn

>+  content/branding/aboutDialog.css               (aboutDialog.css)
>+  content/branding/aboutDialog.css               (aboutDialog.css)

Once is enough :)

Out of curiousity, how does branding for your builds work? Do you edit one of the existing branding directories in-place, or create your own? I'm mainly curious to know how you would typically notice a new requirement introduced on branding packages.
Comment 3 User image Mike Hommey [:glandium] 2011-04-23 01:06:39 PDT
(In reply to comment #2)
> Comment on attachment 527919 [details] [diff] [review]
> Proposal
> 
> >diff --git a/browser/branding/official/content/jar.mn b/browser/branding/official/content/jar.mn
> 
> >+  content/branding/aboutDialog.css               (aboutDialog.css)
> >+  content/branding/aboutDialog.css               (aboutDialog.css)
> 
> Once is enough :)

D'oh.

> Out of curiousity, how does branding for your builds work? Do you edit one of
> the existing branding directories in-place, or create your own? I'm mainly
> curious to know how you would typically notice a new requirement introduced on
> branding packages.

I have my own branding directory, which imports document.png from the unofficial branding, and I have a script that compares the things installed from the unofficial branding with what I install from mine to see if there are files I miss.
That was until the unofficial branding changed, now I'm switching to compare with the official branding.
I wanted to add something to check for missing dtd entities and stuff like that but I haven't come to this yet.
Comment 4 User image Mike Hommey [:glandium] 2011-04-23 01:19:16 PDT
Created attachment 527922 [details] [diff] [review]
Patch

Updated. I will test before landing.
Comment 5 User image Mike Hommey [:glandium] 2011-04-25 23:50:55 PDT
http://hg.mozilla.org/mozilla-central/rev/c5e8cc100248
Comment 6 User image Vlad [QA] 2011-07-28 01:04:46 PDT
Do you have some mockups so I can verify this?
thanks
Comment 7 User image Mike Hommey [:glandium] 2011-07-28 02:15:36 PDT
I don't think there's anything verifiable here, since this doesn't change anything on mozilla builds.

Note You need to log in before you can comment on or make changes to this bug.