1374106 - Stop using the :not(:target) selector in the video element's SVG icons

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: Bug 1374106, part 1 - Remove duplication in the styling of the video elemnent's play/pause button.

Review commit: https://reviewboard.mozilla.org/r/150182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150182/

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: Bug 1374106, part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint.

Review commit: https://reviewboard.mozilla.org/r/150184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150184/

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: Bug 1374106, part 3 - Split up the video control closed captions on/off icon and convert it to use SVG context paint.

Review commit: https://reviewboard.mozilla.org/r/150186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150186/

Comment 4

[Jonathan Watt [:jwatt]]

Comment on attachment 8878938 [details]
Bug 1374106, part 1 - Remove duplication in the styling of the video elemnent's play/pause button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150182/diff/1-2/

Comment 5

[Jonathan Watt [:jwatt]]

Comment on attachment 8878939 [details]
Bug 1374106, part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150184/diff/1-2/

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 8878940 [details]
Bug 1374106, part 3 - Split up the video control closed captions on/off icon and convert it to use SVG context paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150186/diff/1-2/

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8878938 [details]
Bug 1374106, part 1 - Remove duplication in the styling of the video elemnent's play/pause button.

https://reviewboard.mozilla.org/r/150182/#review155788

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8878939 [details]
Bug 1374106, part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint.

https://reviewboard.mozilla.org/r/150184/#review155794

::: toolkit/themes/shared/media/videocontrols.css:166
(Diff revision 2)
>  }
>  
>  .fullscreenButton {
> -  background-image: url(chrome://global/skin/media/fullscreenButton.svg#fullscreen);
> +  background-image: url(chrome://global/skin/media/fullscreenEnterButton.svg);
> +  -moz-context-properties: fill;
> +  fill: white;

please add a new rule for .closedCaptionButton and .fullscreenButton setting both -moz-context-properties and fill

::: toolkit/themes/shared/media/videocontrols.css:169
(Diff revision 2)
> -  background-image: url(chrome://global/skin/media/fullscreenButton.svg#fullscreen);
> +  background-image: url(chrome://global/skin/media/fullscreenEnterButton.svg);
> +  -moz-context-properties: fill;
> +  fill: white;
>  }
>  .fullscreenButton:hover {
> -  background-image: url(chrome://global/skin/media/fullscreenButton.svg#fullscreen-hover);
> +  fill: #48a0f7;

please merge this with the .closedCaptionButton:hover rule

::: toolkit/themes/shared/media/videocontrols.css:172
(Diff revision 2)
>  }
>  .fullscreenButton:hover {
> -  background-image: url(chrome://global/skin/media/fullscreenButton.svg#fullscreen-hover);
> +  fill: #48a0f7;
>  }
>  .fullscreenButton:hover:active {
> -  background-image: url(chrome://global/skin/media/fullscreenButton.svg#fullscreen-active);
> +  fill: #2d89e6;

please merge this with the .closedCaptionButton:hover:active rule

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: Bug 1374106, part 4 - Split up the video control enter/exit fullscreen icon and convert it to use SVG context paint.

Review commit: https://reviewboard.mozilla.org/r/151024/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151024/

Comment 10

[Jonathan Watt [:jwatt]]

Comment on attachment 8878938 [details]
Bug 1374106, part 1 - Remove duplication in the styling of the video elemnent's play/pause button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150182/diff/2-3/

Comment 11

[Jonathan Watt [:jwatt]]

Comment on attachment 8878939 [details]
Bug 1374106, part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150184/diff/2-3/

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 8878940 [details]
Bug 1374106, part 3 - Split up the video control closed captions on/off icon and convert it to use SVG context paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150186/diff/2-3/

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 8878939 [details]
Bug 1374106, part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint.

https://reviewboard.mozilla.org/r/150184/#review156108

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8878940 [details]
Bug 1374106, part 3 - Split up the video control closed captions on/off icon and convert it to use SVG context paint.

https://reviewboard.mozilla.org/r/150186/#review156110

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 8879684 [details]
Bug 1374106, part 4 - Split up the video control enter/exit fullscreen icon and convert it to use SVG context paint.

https://reviewboard.mozilla.org/r/151024/#review156112

::: toolkit/themes/shared/media/videocontrols.css:110
(Diff revision 1)
>  }
>  
>  .playButton:hover:active,
>  .muteButton:hover:active,
> -.closedCaptionButton:hover:active {
> +.closedCaptionButton:hover:active,
> +.fullscreenButton:hover:active{

nit: space before {

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d9b6c6b5a1f
part 1 - Remove duplication in the styling of the video elemnent's play/pause button. r=dao
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e1bc3cb371b
part 2 - Split up the video control mute/unmute/no-audio icon and convert it to use SVG context paint. r=dao
https://hg.mozilla.org/integration/mozilla-inbound/rev/0848cd235cd7
part 3 - Split up the video control closed captions on/off icon and convert it to use SVG context paint. r=dao
https://hg.mozilla.org/integration/mozilla-inbound/rev/c132f4ee269d
part 4 - Split up the video control enter/exit fullscreen icon and convert it to use SVG context paint. r=dao

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0d9b6c6b5a1f
https://hg.mozilla.org/mozilla-central/rev/6e1bc3cb371b
https://hg.mozilla.org/mozilla-central/rev/0848cd235cd7
https://hg.mozilla.org/mozilla-central/rev/c132f4ee269d