Closed Bug 1306007 Opened 3 years ago Closed 3 years ago

Remove srcset/picture feature control preference

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: edgar, Assigned: edgar)

Details

Attachments

(2 files, 3 obsolete files)

Part 2: Drop the unused arguments of TryCreateResponsiveSelector(), v1
3.61 KB, patch
jdm
: review+
Details | Diff | Splinter Review
Part 1: Remove srcset/picture feature control preference, v4, r=jdm
43.24 KB, patch
edgar
: review+
smaug
: review+
Details | Diff | Splinter Review 
picture and srcset features have been enabled since 38 (bug 1017875 and bug 1018389). I think they are stable enough to remove the control preference.
Attachment #8795712 - Attachment is obsolete: true
Attachment #8795713 - Flags: review?(josh)
Attachment #8795716 - Flags: review?(josh)
Attachment #8795713 - Flags: review?(josh) → review+
Comment on attachment 8795716 [details] [diff] [review]
Part 1: Remove srcset/picture feature control preference, v2

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

::: dom/security/test/mixedcontentblocker/test_main.html
@@ +163,5 @@
>  
>    function startTest() {
>      //Set the first set of mixed content settings and increment the counter.
>      //Enable <picture> and <img srcset> for the test.
> +    changePrefs([], function() {

We can remove the changePrefs entirely.
Attachment #8795716 - Flags: review?(josh) → review+
(In reply to Josh Matthews [:jdm] from comment #5)
> Comment on attachment 8795716 [details] [diff] [review]
> Part 1: Remove srcset/picture feature control preference, v2
> 
> Review of attachment 8795716 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/security/test/mixedcontentblocker/test_main.html
> @@ +163,5 @@
> >  
> >    function startTest() {
> >      //Set the first set of mixed content settings and increment the counter.
> >      //Enable <picture> and <img srcset> for the test.
> > +    changePrefs([], function() {
> 
> We can remove the changePrefs entirely.

changePrefs also adds some base preference [1], like "security.mixed_content.block_display_content", we can not remove it entirely.
[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/test/mixedcontentblocker/test_main.html#23
Comment on attachment 8795716 [details] [diff] [review]
Part 1: Remove srcset/picture feature control preference, v2

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

::: dom/security/test/mixedcontentblocker/test_main.html
@@ +162,5 @@
>    }
>  
>    function startTest() {
>      //Set the first set of mixed content settings and increment the counter.
>      //Enable <picture> and <img srcset> for the test.

Ah, this comment could be removed.
Carrying forward r+ and addressing comment #7.
Attachment #8795716 - Attachment is obsolete: true
Attachment #8796413 - Flags: review+
Keywords: checkin-needed
Part 1 doesn't apply cleanly. Please rebase.
Flags: needinfo?(echen)
Keywords: checkin-needed
Rebase
Attachment #8796413 - Attachment is obsolete: true
Flags: needinfo?(echen)
Attachment #8796846 - Flags: review+
Comment on attachment 8796846 [details] [diff] [review]
Part 1: Remove srcset/picture feature control preference, v4, r=jdm

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

This patch touches WebIDL files, so it requires review from DOM peer.
Could you help to review webidl changes, Olli?
Thank you.
Attachment #8796846 - Flags: review?(bugs)
Attachment #8796846 - Flags: review?(bugs) → review+
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/82d198b87307
Part 1: Remove srcset/picture feature control preference; r=jdm,smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f435a5a9ad0
Part 2: Drop the unused arguments of TryCreateResponsiveSelector(); r=jdm
https://hg.mozilla.org/mozilla-central/rev/82d198b87307
https://hg.mozilla.org/mozilla-central/rev/4f435a5a9ad0
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.