CSS warning when the account provisioner is displayed

RESOLVED FIXED in Thunderbird 14.0

Status

Thunderbird
Account Manager
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: florian, Assigned: aceman)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 14.0
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Warning: Unknown pseudo-class or pseudo-element 'placeholder'.  Ruleset ignored due to bad selector.
Source File: chrome://messenger/skin/newmailaccount/accountProvisioner.css
Line: 224

Line 224 is:
#search input[type="text"]:placeholder {

The same code exists in all 3 themes:
/mail/themes/qute/mail/newmailaccount/accountProvisioner.css
/mail/themes/pinstripe/mail/newmailaccount/accountProvisioner.css
/mail/themes/gnomestripe/mail/newmailaccount/accountProvisioner.css

I think ":placeholder" just needs to be replaced with ":-moz-placeholder", see https://developer.mozilla.org/en/CSS/:-moz-placeholder
(Reporter)

Comment 1

5 years ago
Aceman, not sure if you are looking for more easy cleanup bugs, but if my analysis in comment 0 is right and it isn't filed anywhere else yet with a patch waiting for review, this seems easy; so CC'ing you just in case :-).
(Reporter)

Comment 2

5 years ago
Possibly caused by bug 700536 and related to bug 709945.
(Assignee)

Comment 3

5 years ago
Thanks,
it is in 4 files:
http://mxr.mozilla.org/comm-central/search?string=%3Aplaceholder&find=.css&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

No other occurences in comm-central.
I couldn't find a dupe. I'll do it.
I just usually don't care much for new experimental modules :)
Assignee: nobody → acelists
Blocks: 709945
(Assignee)

Comment 4

5 years ago
Created attachment 609392 [details] [diff] [review]
patch

This makes the change and the warning does not happen. But I can't see any visual change.
Attachment #609392 - Flags: ui-review?(bwinton)
Attachment #609392 - Flags: review?(bwinton)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Comment on attachment 609392 [details] [diff] [review]
patch

Stealing this review request.
Attachment #609392 - Flags: review?(bwinton) → review?(mconley)
Comment on attachment 609392 [details] [diff] [review]
patch

Review of attachment 609392 [details] [diff] [review]:
-----------------------------------------------------------------

This looks right to me!  Thanks aceman!
Attachment #609392 - Flags: review?(mconley) → review+
(Assignee)

Comment 7

5 years ago
Thanks to florian:)
Keywords: checkin-needed
Created attachment 609790 [details]
Comparison of having -moz-placeholder, or no rule at all...

Hm.  So I'm reconsidering my r+ on the patch for this.

While the code looks good, I'm not sure the rule actually gives us anything that we want.  The difference between having -moz-placeholder vs. just tearing that rule out seem negligible.  See attached.
I'm going to take off checkin-needed for now until we resolve whether or not we even need the rule.

bwinton: do we even need this rule?  If we were fine with the way Account Provisioner already was, we might as well excise it, since I don't think it's adding much.

-Mike
Keywords: checkin-needed
Comment on attachment 609392 [details] [diff] [review]
patch

Taking off my r+ until we determine whether or not we even need the rule.
Attachment #609392 - Flags: review+
(Assignee)

Comment 11

5 years ago
That is what I said in comment 4 (no visual change) :)
Comment on attachment 609392 [details] [diff] [review]
patch

Yeah, I think we can safely get rid of that rule, since it doesn't really add anything.

Thanks,
Blake.
Attachment #609392 - Flags: ui-review?(bwinton) → ui-review-
(Assignee)

Comment 13

5 years ago
Created attachment 611520 [details] [diff] [review]
patch v2
Attachment #609392 - Attachment is obsolete: true
Attachment #611520 - Flags: ui-review?(bwinton)
Comment on attachment 611520 [details] [diff] [review]
patch v2

Looks good, so I'll say ui-r=me, and r=me while I'm here.

Thanks,
Blake.
Attachment #611520 - Flags: ui-review?(bwinton)
Attachment #611520 - Flags: ui-review+
Attachment #611520 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
http://hg.mozilla.org/comm-central/rev/3c7492c88129
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
You need to log in before you can comment on or make changes to this bug.