Closed Bug 459372 Opened 11 years ago Closed 11 years ago

Move filefield style rules to toolkit

Categories

(Toolkit :: Preferences, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: alfredkayser, Assigned: neil)

References

Details

(Keywords: fixed1.9.1)

Attachments

(2 files)

Followup to bug 455880 and bug 443410:

The .css part of filefield should still be moved from /browser/skin/preferences/... to /mozapps/skin/preferences/preferences.css or even to /global/skin/preferences.css?

And there are other general preference styles that can be centralized also.
No, the .css part of filefield should be moved to global/skin/filefield.css

For pinstripe, ben checked in some bogus rules and nobody even noticed...

Note that SeaMonkey's port includes some optimisations/improvements:
* the colour on the .fileFieldContentBox is already correct
* the 0.5 opacity on the disabled .fileFieldContentBox is unnecessary
* the 0.2 opacity on the disabled .fileFieldIcon is too faint (now 0.4)
* the margin on the .fileFieldContentBox and the padding override on the .fileFieldLabel are unnecessary (adjusted .fieldFieldIcon margin to compensate)
Even better!
Duplicate of this bug: 443512
Attached patch Proposed patchSplinter Review
I did the best I could for pinstripe, given the circumstances...
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #342801 - Flags: review?(mano)
I am not sure whether this patch is right.
It removes everything but fileField from preferences.css.
It think it was intended the other way round: fileField styles to filefield.css and the remainder to remain in preferences.css.
(In reply to comment #5)
> I am not sure whether this patch is right.
> It removes everything but fileField from preferences.css.
> It think it was intended the other way round: fileField styles to filefield.css
> and the remainder to remain in preferences.css.
If you look closely, it copies preferences.css to filefield.css and then removes all the "wrong" styles from both files.
Ah, the 'diff' view doesn't show this (the raw view does, but is much more difficult to review).
Comment on attachment 342801 [details] [diff] [review]
Proposed patch

Sorry for the much delayed review.
Attachment #342801 - Flags: review?(mano) → review+
Pushed changeset ebabcc8e40e7 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This is a follow-up to bug 455880 which is already in 1.9.1 that makes things easier for theme authors skinning multiple apps as these styles are moved into a central location instead of duplicated for each app (note that no bugs have yet been filed on the apps in question as they are all based on 1.9.1).
Attachment #351171 - Flags: approval1.9.1?
Does this patch imply late-compat? We've promised not to make any changes that would affect themers post-beta-2.
(In reply to comment #11)
> Does this patch imply late-compat? We've promised not to make any changes that
> would affect themers post-beta-2.
It won't break existing themes as the only users of the filefield were in preferences anyway.
Comment on attachment 351171 [details] [diff] [review]
Patch as pushed (fixed merge conflicts)

a191=beltzner
Attachment #351171 - Flags: approval1.9.1? → approval1.9.1+
Pushed changeset e11d74941efe to releases/mozilla-1.9.1 (with yet more merge conflicts fixed, sigh...)
Keywords: fixed1.9.1
Blocks: 513891
Blocks: 513893
Blocks: 514830
Depends on: 1191778
You need to log in before you can comment on or make changes to this bug.