Closed Bug 1218977 Opened 4 years ago Closed 4 years ago

Pocket toolbar panel should have rounded corners

Categories

(Firefox :: Pocket, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: sevaan, Assigned: Gijs)

References

Details

Attachments

(2 files, 1 obsolete file)

The panel for Pocket has sharp corners, but they should be rounded to match with the other panels in Firefox.
Bug 1199050 might be helpful in fixing this.  (If it works, it's a pretty minimal change… :)
Attached patch fix panel corners (obsolete) — Splinter Review
Assignee: nobody → mixedpuppy
Attachment #8722670 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8722670 [details] [diff] [review]
fix panel corners

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

Is there a reason not to just add:

.cui-widget-panel > .panel-arrowcontainer > .panel-arrowcontent,

to this rule:

https://dxr.mozilla.org/mozilla-central/rev/789a12291942763bc1e3a89f97e0b82dc1c9d00b/browser/themes/shared/customizableui/panelUIOverlay.inc.css#201-204

instead? That would need more testing, obviously, but I presume that it would be a more correct fix... Why don't the other panels (try e.g. the dev tools panel or the history one) have this issue right now?
Attachment #8722670 - Flags: review?(gijskruitbosch+bugs)
Your suggestion doesn't work. It's because there is an iframe inside the panel.  Some css somewhere causes the iframe to overflow, thus pointy corners.  Presumably it overrides the css you pointed at.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Shane Caraveo (:mixedpuppy) from comment #4)
> Your suggestion doesn't work. It's because there is an iframe inside the
> panel.  Some css somewhere causes the iframe to overflow, thus pointy
> corners.  Presumably it overrides the css you pointed at.

I'm confused. Your patch's selector is:

 .cui-widget-panel[viewId="PanelUI-pocketView"] > .panel-arrowcontainer > .panel-arrowcontent

Mine is the same, except for the viewId attribute selector. It is possible, but surprising, that that would make a difference.

I just tried my suggestion from the Style Editor on Nightly, and it seems to WFM on OS X.

What am I missing?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mixedpuppy)
indeed.  very strange, I retested just now and it worked fine.
Flags: needinfo?(mixedpuppy)
Attachment #8722670 - Attachment is obsolete: true
Attachment #8723162 - Flags: review?(gijskruitbosch+bugs)
Attachment #8723162 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/a951d792d9b0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Depends on: 1251843
Reproduced this according to firefox 44.0a1(2015-10-27) 

It's verified on Latest Nightly
Nightly--Build ID(20160302030209), Mozilla/5.0 (Windows NT 6.3; rv:47.0) Gecko/20100101 Firefox/47.0

Tested OS-- Windows 8.1 32bit
QA Whiteboard: [bugday-20160302]
This isn't actually a regression from pocket becoming an add-on, as I thought, but was implemented in bug 1161793 because it reduced the top and bottom padding to 0. That should have been accompanied by adding padding and matching corner rounding to the inner elements.

The fix (that I helped suggest) in this bug is therefore wrong, and it causes bug 1251843, so I'm going to back it out. Then we can choose to fix this separately, but considering we've shipped this for a whole series of releases now, I don't know that it should actually be a priority.
Blocks: 1161793
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Gijs, what do you think about just using the earlier patch in Attachment #8722670 [details] [diff] ?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Shane Caraveo (:mixedpuppy) from comment #13)
> Gijs, what do you think about just using the earlier patch in Attachment
> #8722670 [details] [diff] ?

I mean, it's still not really the right fix.

It shouldn't be particularly hard to fix the border-radius on the bottom and top of the elements in question to match on OS X. Is there some reason that's not an option?
Flags: needinfo?(gijskruitbosch+bugs)
I'm just going to steal it. Hope you don't mind! :-)
Assignee: mixedpuppy → gijskruitbosch+bugs
Status: REOPENED → ASSIGNED
Comment on attachment 8726968 [details]
MozReview Request: Bug 1218977 - pocket panel should have rounded corners on OS X, r?mixedpuppy

https://reviewboard.mozilla.org/r/38343/#review34855
Attachment #8726968 - Flags: review?(mixedpuppy) → review+
Hm, this won't do, it needs to be specific to pocket being in its own panel.
Comment on attachment 8726968 [details]
MozReview Request: Bug 1218977 - pocket panel should have rounded corners on OS X, r?mixedpuppy

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38343/diff/1-2/
hmm, thats an odd comment via reviewboard.  r+
https://hg.mozilla.org/mozilla-central/rev/209b08d0d9b1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 47 → Firefox 48
QA Whiteboard: [bugday-20160302] → [bugday-20160302] [good first verify]
You need to log in before you can comment on or make changes to this bug.