459372 - Move filefield style rules to toolkit

Status: RESOLVED FIXED

(Toolkit :: Preferences, defect)

Description

[Alfred Kayser]

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.

Comment 1

[[e-mail]]

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)

Comment 2

[Alfred Kayser]

Even better!

Comment 4

[[e-mail]]

Attachment: Proposed patch

I did the best I could for pinstripe, given the circumstances...

Comment 5

[Alfred Kayser]

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.

Comment 6

[[e-mail]]

(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.

Comment 7

[Alfred Kayser]

Ah, the 'diff' view doesn't show this (the raw view does, but is much more difficult to review).

Comment 8

[Asaf Romano (gone)]

Comment on attachment 342801 [details] [diff] [review]
Proposed patch

Sorry for the much delayed review.

Comment 9

[[e-mail]]

Pushed changeset ebabcc8e40e7 to mozilla-central.

Comment 10

[[e-mail]]

Attachment: Patch as pushed (fixed merge conflicts)

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).

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Does this patch imply late-compat? We've promised not to make any changes that would affect themers post-beta-2.

Comment 12

[[e-mail]]

(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 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351171 [details] [diff] [review]
Patch as pushed (fixed merge conflicts)

a191=beltzner

Comment 14

[[e-mail]]

Pushed changeset e11d74941efe to releases/mozilla-1.9.1 (with yet more merge conflicts fixed, sigh...)