Closed Bug 857536 Opened 8 years ago Closed 8 years ago

Cleanup the CSS properties applied on <input type='file'> from forms.css

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23
Tracking Status
firefox21 --- unaffected
firefox22 --- fixed
firefox23 --- fixed
firefox24 --- fixed

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
We have some properties applying on |input|, some on |input[type='file']| and for some reasons, we don't revert all of them as expected. This should cleanup most things. It will make some bugs like border and background being ignored more visible because they will no longer apply to the label. They should apply to the element. This is another bug.

It should fixes the fact that the label had a background by default (inherited from a |input| property).
Attachment #732767 - Flags: review?(bzbarsky)
Comment on attachment 732767 [details] [diff] [review]
Patch

>+  background-color: none;

This isn't valid CSS.  You meant "transparent", right?

r=me with that.
Attachment #732767 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a6c83f2499e
Flags: in-testsuite+
Target Milestone: --- → mozilla23
(In reply to Boris Zbarsky (:bz) from comment #1)
> Comment on attachment 732767 [details] [diff] [review]
> Patch
> 
> >+  background-color: none;
> 
> This isn't valid CSS.  You meant "transparent", right?

Yes, and that was fixed by the next patch in my queue actually.
> Yes, and that was fixed by the next patch in my queue actually.

Er... yes, but why did you check in the broken code, which will cause spammy warnings, in anyway?
... I *again* forgot to qref before importing in my m-i repository :( Will push a follow-up.
Oh no. I actually just forgot to update the test. That's less important I guess. My next patch should fix it anyway.
https://hg.mozilla.org/mozilla-central/rev/2a6c83f2499e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 732767 [details] [diff] [review]
Patch

[Approval Request Comment]
This is required to push bug 52500. I thought this patch was landed earlier.
The user impact is similar to bug 52500 then and the risk is also similar. In the way that this patch isn't more risky than the one in bug 52500 and the user impact isn't going to be more than not able to psuh bug 52500 if this patch don't get uplifted.

(FWIW, it is in Aurora)
Attachment #732767 - Flags: approval-mozilla-beta?
Attachment #732767 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Given the automation coverage for this bug, is there anything else QA can do to manually verify this? If so, please provide some guidelines.
(In reply to Mihaela Velimiroviciu [QA] (:mihaelav) from comment #10)
> Given the automation coverage for this bug, is there anything else QA can do
> to manually verify this? If so, please provide some guidelines.

I think we are good here. Thank you :)
You need to log in before you can comment on or make changes to this bug.