The default bug view has changed. See this FAQ.

Remove dropped prefixes from comm-central

RESOLVED FIXED in Thunderbird 14.0

Status

Thunderbird
Theme
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 14.0
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

(thunderbird13 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

5 years ago
Bug 693510 dropped the support of -moz-border-radius and -moz-box-shadow. Comm-central has still some of this properties.
(Assignee)

Comment 1

5 years ago
Created attachment 600559 [details] [diff] [review]
remove the prefixes

This patch removes the prefixes. It has still some parts with the prefixes: testpilot, venkman, inspector and irc. Also mozmill uses this prefixes.

I wasn't sure if I should also remove them so I leaved them. Should I expand the patch and fix also these parts?
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #600559 - Flags: review?(bwinton)
(Assignee)

Comment 2

5 years ago
I have also a small part of calendar in this patch. I hope it's okay like this.
(Assignee)

Comment 3

5 years ago
Created attachment 600631 [details] [diff] [review]
remove the prefixes - full patch

This is a full patch with testpilot and mozmill. The in mxr listed entries under /mozilla (venkman, irc and inspector) aren't in my clone.

I don't know what happens with the tests when I change the properties in them.

I leave the previous patch active. Then you can decide which one should be used (if one would be used)
Attachment #600631 - Flags: review?(bwinton)

Comment 4

5 years ago
> (venkman, irc and inspector) aren't in my clone
They are separate nested repositories. You'll need to file separate bugs for these, if there aren't already.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 731999
(Assignee)

Comment 6

5 years ago
Created attachment 602393 [details] [diff] [review]
remove the prefixes

Patch updated after landing of Bug 730930
Attachment #600559 - Attachment is obsolete: true
Attachment #600631 - Attachment is obsolete: true
Attachment #602393 - Flags: review?(bwinton)
Attachment #600559 - Flags: review?(bwinton)
Attachment #600631 - Flags: review?(bwinton)
Comment on attachment 602393 [details] [diff] [review]
remove the prefixes

You missed the following:

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/content/screen.css
196:	-moz-border-radius: 0.25em;

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/skin/all/css/screen-standalone-mobile.css
249:	-moz-border-radius: 0.25em;

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/skin/all/css/screen-standalone.css
232:	-moz-border-radius: 0.25em;

mail/test/resources/mozmill/mozmill/extension/content/css/mozmill.css
42:  -moz-border-radius: 1px;

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/content/screen.css
197:	-webkit-border-radius: 0.25em;

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/skin/all/css/screen-standalone-mobile.css
250:	-webkit-border-radius: 0.25em;

mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/skin/all/css/screen-standalone.css
233:	-webkit-border-radius: 0.25em;

> +++ b/mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/content/browser.css
> @@ -214,7 +214,7 @@
> -    -moz-border-radius: 100%;
> +    border-radius: 10000px;

Why did you change this from 100% to 10000px?

I don't think we should change "jquery-ui-1.7.1.custom.css", anything in the "bespin" directory, or anything in "virtualenv/docs".

So given all of that, I'm gonna say r- for now, but I look forward to the next version.

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

Comment 8

5 years ago
Created attachment 603435 [details] [diff] [review]
remove the prefixes

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #7)
> Comment on attachment 602393 [details] [diff] [review]
> remove the prefixes
> 
The missed -moz-border-radius and -webkit-border-radius where in not used parts (commented out) -> fixed.

> > +++ b/mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/content/browser.css
> > @@ -214,7 +214,7 @@
> > -    -moz-border-radius: 100%;
> > +    border-radius: 10000px;
> 
> Why did you change this from 100% to 10000px?

I thought this was missed when the behavior of border-radius was changed. But also Firefox has this. So I let the 100% now.

> I don't think we should change "jquery-ui-1.7.1.custom.css", anything in the
> "bespin" directory, or anything in "virtualenv/docs".

Removed from patch
Attachment #602393 - Attachment is obsolete: true
Attachment #603435 - Flags: review?(bwinton)

Comment 9

5 years ago
There used to be a bug in the computation of percentage border radius. The old computation calculated the percentage of the width of the border box, and applied this to the height too. If the height was smaller than the width, then it would apply the rules for radii which are too big, which is to reduce them in proportion, thus it would create a radius using the height instead. The net effect (assuming that the border radius is applied to all corners) is that you would get a rectangle with semicircular end caps.

The new computation calculates the percentage of the width and height independently, resulting in an ellipse (again assuming that the border radius is applied to all corners).
Comment on attachment 603435 [details] [diff] [review]
remove the prefixes

Okay, I like it.  But I'm also convinced by Neil's explanation, so we should probably re-add that 10000px.  (Or show me a screenshot of each of the options, and I'll pick.  Or you can pick.  :)

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

Comment 11

5 years ago
Created attachment 607731 [details]
Badge Comparison. Left 100% right 10000px

This is the comparison between border-radius: 100% and 10000px. I would say the left badge with small number looks with small numbers than the right with radius 10000px. With bigger numbers (here 20) almost no difference.

I propose to leave 100%.
Works for me!
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

5 years ago
Comment on attachment 603435 [details] [diff] [review]
remove the prefixes

[Approval Request Comment]
The prefix drop was before the merge and it would be good if this can land in aurora. It's all CSS, ergo low risk. If landed we can prevent bug reports about missing radii.
Attachment #603435 - Flags: approval-comm-aurora?
http://hg.mozilla.org/comm-central/rev/a3d01125cea4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Attachment #603435 - Flags: approval-comm-aurora? → approval-comm-aurora+
Checked in:

http://hg.mozilla.org/releases/comm-aurora/rev/af3f77dda5cf
status-thunderbird13: --- → fixed
You need to log in before you can comment on or make changes to this bug.