Closed
Bug 1291013
Opened 8 years ago
Closed 8 years ago
Closed-caption button spacing is asymetric
Categories
(Toolkit :: Video/Audio Controls, defect, P2)
Toolkit
Video/Audio Controls
Tracking
()
VERIFIED
FIXED
mozilla51
People
(Reporter: Dolske, Assigned: ralin)
References
Details
Attachments
(2 files)
20.42 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
jaws
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
The closed-caption button on the default video control has strange spacing -- there are 7 pixels of space between it and the volume control on the left, and 20 pixels between it and the full-screen control on the right. The volume control has 13 pixels between it and the mute button on its left. (Those are hidpi pixels at 2x, so 1/2 that for normal CSS pixels).
Screenshot attached, using https://www.iandevlin.com/html5test/webvtt/html5-video-webvtt-sample.html as a demo page
Comment 1•8 years ago
|
||
Ray, can you take this bug? We should get this fixed in Firefox 50 since it should be a simple fix.
[Tracking Requested - why for this release]: Visible UI blurriness in new feature shipping in v50.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Flags: needinfo?(ralin)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/68542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68542/
Attachment #8776877 -
Flags: review?(jaws)
Assignee | ||
Comment 3•8 years ago
|
||
btw, fullscreen button is no longer the only button on the volume control's right, so move the margin from volumeStack to controlBar's padding.
Comment 4•8 years ago
|
||
Comment on attachment 8776877 [details]
Bug 1291013 - Adjust closed caption button spacing.
Please rebase this on top of the patch that I put in bug 1291268
Attachment #8776877 -
Flags: review?(jaws)
Updated•8 years ago
|
Priority: -- → P2
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8776877 [details]
Bug 1291013 - Adjust closed caption button spacing.
https://reviewboard.mozilla.org/r/68542/#review68100
::: toolkit/themes/shared/media/videocontrols.css:70
(Diff revision 2)
> .muteButton[noAudio] + .volumeStack {
> display: none;
> }
>
> .closedCaptionButton {
> - background-image: url(chrome://global/skin/media/closeCaptionButton.png);
> + background: url(chrome://global/skin/media/closeCaptionButton.png) 4px;
Please don't use shorthand here. Keep the background-image property and then use a separate property for any other values you want to set.
::: toolkit/themes/shared/media/videocontrols.css:419
(Diff revision 2)
> .muteButton[noAudio] {
> background-image: url(chrome://global/skin/media/noAudio@2x.png);
> background-size: 33px 28px;
> }
> - .closeCaptionButton {
> - background-image: url(chrome://global/skin/media/closeCaptionButton@2x.png);
> + .closedCaptionButton {
> + background: url(chrome://global/skin/media/closeCaptionButton@2x.png) 4px;
Same comment here about shorthand properties.
Attachment #8776877 -
Flags: review?(jaws) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0e721493391d
Adjust closed caption button spacing. r=jaws
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hi Ray, since this is a regression in Fx50 and fixed in Nightly, could you please nominate a patch for uplift to Aurora? Thanks!
Flags: needinfo?(ralin)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8776877 [details]
Bug 1291013 - Adjust closed caption button spacing.
Approval Request Comment
[Feature/regressing bug #]: fix wrong spacing of closed caption button
[User impact if declined]: weird spacing between controls' buttons
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low risk. adjusting style only.
[String/UUID change made/needed]: none
Flags: needinfo?(ralin)
Attachment #8776877 -
Flags: approval-mozilla-aurora?
Comment on attachment 8776877 [details]
Bug 1291013 - Adjust closed caption button spacing.
Low risk fix for a recent regression, Aurora50+
Attachment #8776877 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi Justin, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(dolske)
Comment 14•8 years ago
|
||
bugherder uplift |
Reporter | ||
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Flags: needinfo?(dolske)
You need to log in
before you can comment on or make changes to this bug.
Description
•