Closed Bug 1268749 Opened 7 years ago Closed 7 years ago

Add a pref to disable unprefixed Fullscreen API

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
firefox46 --- unaffected
firefox47 + fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: btpp-active)

Attachments

(7 files, 1 obsolete file)

MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug
58 bytes, text/x-review-board-request
smaug
: review+
lizzard
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details
MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam
58 bytes, text/x-review-board-request
heycam
: review+
Details
MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam
58 bytes, text/x-review-board-request
heycam
: review+
Details
MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug
58 bytes, text/x-review-board-request
smaug
: review+
Details
MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug
58 bytes, text/x-review-board-request
smaug
: review+
Details
MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug
58 bytes, text/x-review-board-request
smaug
: review+
Details
followup - Avoid querying :fullscreen selector for context menu
3.76 KB, patch
xidorn
: review+
Details | Diff | Splinter Review 
Microsoft recently reached out to us and talked about some issues they found with unprefixed Fullscreen API, and the plan that they are going to disable that by default with the next Edge release.

I discussed with Olli and Ehsan about this offline today, and they think Beta is not the right place for us to investigate this kind of breakage, and we should step back and disable it by default for Beta and Release, then wait for further information.

This bug is for adding the pref and disable it for Beta and Release.
[Tracking Requested - why for this release]: Because this is the release we initially unprefix this API.
status-firefox46: --- → unaffected
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox49: --- → affected
tracking-firefox47: --- → ?
Summary: Add a pref to disable unprefixed Fullscreen API and disable it on non-release versions → Add a pref to disable unprefixed Fullscreen API
Blocks: 1268797
Whiteboard: btpp-active
Blocks: 1269276
try push looks fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9db611754533

Note that this try push commit disables unprefixed Fullscreen API by default (ignoring the version), so it should be more similiar to what we would see in release versions.
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49963/diff/1-2/
Comment on attachment 8747615 [details]
MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49965/diff/1-2/
Comment on attachment 8747616 [details]
MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49967/diff/1-2/
Comment on attachment 8747617 [details]
MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49969/diff/1-2/
Comment on attachment 8747618 [details]
MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49971/diff/1-2/
Comment on attachment 8747619 [details]
MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49973/diff/1-2/
Attachment #8747614 - Flags: review?(bugs) → review+
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

https://reviewboard.mozilla.org/r/49963/#review46989
Comment on attachment 8747617 [details]
MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug

https://reviewboard.mozilla.org/r/49969/#review46991
Attachment #8747617 - Flags: review?(bugs) → review+
Comment on attachment 8747618 [details]
MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug

https://reviewboard.mozilla.org/r/49971/#review46995

Those fixed, and _tested_, r+

::: dom/events/EventListenerManager.cpp:288
(Diff revision 2)
>    listener->mTypeAtom = aTypeAtom;
>    listener->mFlags = aFlags;
>    listener->mListenerIsHandler = aHandler;
>    listener->mHandlerIsString = false;
>    listener->mAllEvents = aAllEvents;
> +  listener->mIsChrome = NS_IsMainThread() &&

NS_IsMainThread() check is a bit slow, and you have that information already in mIsMainThreadELM member variable in EventListenerManager, so use that please.

::: dom/events/EventListenerManager.cpp:289
(Diff revision 2)
>    listener->mFlags = aFlags;
>    listener->mListenerIsHandler = aHandler;
>    listener->mHandlerIsString = false;
>    listener->mAllEvents = aAllEvents;
> +  listener->mIsChrome = NS_IsMainThread() &&
> +                        (!nsContentUtils::GetCurrentJSContext() ||

I would use here LegacyIsCallerChromeOrNativeCode

::: dom/events/EventListenerManager.cpp:685
(Diff revision 2)
>      if (mIsMainThreadELM) {
>        return aListener->mTypeAtom == aEvent->mSpecifiedEventType;
>      }
>      return aListener->mTypeString.Equals(aEvent->mSpecifiedEventTypeString);
>    }
> +  if (!nsContentUtils::IsUnprefixedFullscreenApiEnabled() &&

This breaks dispatching fullscreenchange or fullscreenerror using scripts in the page, or more exactly, this breaks page to handle them.

But adding a check that if event isn't trusted, then listener should be called, then this should be fine.

So, before the nsContentUtils check,  add
aEvent->IsTrusted() &&

Also, please test at least manually that dispatching those events from web page works.
Attachment #8747618 - Flags: review?(bugs) → review+
Comment on attachment 8747619 [details]
MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug

https://reviewboard.mozilla.org/r/49973/#review46997

::: dom/html/test/file_fullscreen-unprefix-disabled-inner.html:69
(Diff revision 2)
> +  document.removeEventListener("mozfullscreenerror", errorFullscreen);
> +  // Wait a short time before exiting this test to confirm that there is
> +  // really no unwanted event gets dispatched.
> +  setTimeout(() => opener.finish(), 200);
> +}
> +

Please add a check to this test that dispatching fullscreenchange and fullscreenerror events from scripts and getting listeners called still works.
Attachment #8747619 - Flags: review?(bugs) → review+
Comment on attachment 8747615 [details]
MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam

https://reviewboard.mozilla.org/r/49965/#review47139

We should have similar static_asserts to those in nsCSSProps.cpp that check that if ENABLED_IN_CHROME is set, ENABLED_IN_UA_SHEETS is also set.

::: layout/style/nsCSSParser.cpp:5917
(Diff revision 2)
>    CSSPseudoElementType pseudoElementType =
>      nsCSSPseudoElements::GetPseudoType(pseudo);
>    CSSPseudoClassType pseudoClassType =
> -    nsCSSPseudoClasses::GetPseudoType(pseudo);
> +    nsCSSPseudoClasses::GetPseudoType(pseudo, AgentRulesEnabled(),
> +                                      ChromeRulesEnabled());

I think having the method take two booleans can make it hard to understand what the arguments are, and makes it easier to get the order of argument passing wrong.

I think we should be consistent with the way we check for CSS properties being enabled.  So instead of using two booleans here, we should use nsCSSProps::EnabledState (maybe lifted out to a top-level type).  And we should probably use the same design for nsCSSPeudoElements::GetPseudoType too.

This can be a followup.

::: layout/style/nsCSSPseudoClasses.h:13
(Diff revision 2)
>  #ifndef nsCSSPseudoClasses_h___
>  #define nsCSSPseudoClasses_h___
>  
>  #include "nsStringFwd.h"
>  
> -// This pseudo-class is accepted only in UA style sheets.
> +// The following two flags along with the pref defines where the this

s/where the/where/ (I see that typo in nsCSSProps.h, too).
Attachment #8747615 - Flags: review?(cam) → review+
Comment on attachment 8747616 [details]
MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam

https://reviewboard.mozilla.org/r/49967/#review47143
Attachment #8747616 - Flags: review?(cam) → review+
Blocks: 1269975
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49963/diff/2-3/
Attachment #8747614 - Attachment description: MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r?smaug → MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug
Attachment #8747615 - Attachment description: MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r?heycam → MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam
Attachment #8747616 - Attachment description: MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r?heycam → MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam
Attachment #8747617 - Attachment description: MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r?smaug → MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug
Attachment #8747618 - Attachment description: MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r?smaug → MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug
Attachment #8747619 - Attachment description: MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r?smaug → MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug
Comment on attachment 8747615 [details]
MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49965/diff/2-3/
Comment on attachment 8747616 [details]
MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49967/diff/2-3/
Comment on attachment 8747617 [details]
MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49969/diff/2-3/
Comment on attachment 8747618 [details]
MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49971/diff/2-3/
Comment on attachment 8747619 [details]
MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49973/diff/2-3/
https://reviewboard.mozilla.org/r/49965/#review47139

> I think having the method take two booleans can make it hard to understand what the arguments are, and makes it easier to get the order of argument passing wrong.
> 
> I think we should be consistent with the way we check for CSS properties being enabled.  So instead of using two booleans here, we should use nsCSSProps::EnabledState (maybe lifted out to a top-level type).  And we should probably use the same design for nsCSSPeudoElements::GetPseudoType too.
> 
> This can be a followup.

Followup filed bug 1269975 and bug 1269976.
Comment on attachment 8747615 [details]
MozReview Request: Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49965/diff/3-4/
Comment on attachment 8747616 [details]
MozReview Request: Bug 1268749 part 3 - Hide :fullscreen pseudo-class from content when unprefixed API is disabled. r=heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49967/diff/3-4/
Comment on attachment 8747617 [details]
MozReview Request: Bug 1268749 part 4 - Hide unprefixed Fullscreen API from content when disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49969/diff/3-4/
Comment on attachment 8747618 [details]
MozReview Request: Bug 1268749 part 5 - Avoid dispatching unprefixed fullscreen events to content if unprefixed API is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49971/diff/3-4/
Comment on attachment 8747619 [details]
MozReview Request: Bug 1268749 part 6 - Add test for behavior when unprefixed api is disabled. r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49973/diff/3-4/
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

Approval Request Comment
[Feature/regressing bug #]: bug 743198, unprefix Fullscreen API
[User impact if declined]: may see webcompat issue (see the blockers of bug 1269276)
[Describe test coverage new/current, TreeHerder]: new test is added for the behavior when unprefixed fullscreen api is disabled
[Risks and why]: this is a non-trivial change, so it could be somehow risky.
[String/UUID change made/needed]: n/a
Attachment #8747614 - Flags: approval-mozilla-beta?
Attachment #8747614 - Flags: approval-mozilla-aurora?
All six patches are needed to uplift.
The webcompat issues are also tracked in:
* https://github.com/webcompat/web-bugs/issues/2498 (could be mitigated by bug 1268794)
* https://github.com/webcompat/web-bugs/issues/2499 (need website fix)
and some other related spec changes are in:
* https://github.com/whatwg/fullscreen/issues/2 (bug 1268798)
* https://github.com/whatwg/fullscreen/issues/38 (bug 1269157)
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

Let's put this in aurora 48 in preparation for possible uplift to beta.
Sounds like it is important that we be able to test disabling this api on beta as soon as we can.
Attachment #8747614 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Part 1 applied cleanly.
Part 2 hit the following:

grafting 341511:04e6d36ccc44 "Bug 1268749 part 2 - Make pseudo-classes able to present conditionally like properties. r=heycam"
merging layout/inspector/inDOMUtils.cpp                                                                                                                   
merging layout/style/nsCSSParser.cpp                                                                                                                  
merging layout/style/nsCSSPseudoClassList.h                                                                                                           
merging layout/style/nsCSSPseudoClasses.cpp                                                                                                           
merging layout/style/nsCSSPseudoClasses.h                                                                                                             
warning: conflicts while merging layout/inspector/inDOMUtils.cpp! (edit, then use 'hg resolve--mark')                                                                         
warning: conflicts while merging layout/style/nsCSSParser.cpp! (edit, then use 'hg resolve --mark')                                                                            
warning: conflicts while merging layout/style/nsCSSPseudoClasses.cpp! (edit, then use 'hg resolve --mark')                                                                     
abort: unresolved conflicts, can't continue 


Not sure if any of the other patches need rebasing after that.
Flags: needinfo?(bugzilla)
I'll look into this next Monday. I wonder whether I can push to mozilla-aurora directly myself? It feels to me submitting the rebased patchset for aurora and beta isn't very efficient.
Flags: needinfo?(bugzilla) → needinfo?(wkocher)
I shouldn't have cancelled my ni?...
Flags: needinfo?(bugzilla)
You can probably carry forward the the a+ and land them yourself if you'd prefer.
Flags: needinfo?(wkocher)
The conflict is from a renaming in bug 1250820.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #43)
> Pulsebot does not log changesets automatically for pushes to mozilla-aurora?
Correct. Landing to aurora usually means status flags need to get changed, and pulsebot isn't smart enough to try setting those, so just doesn't bother commenting at all.
Comment on attachment 8747614 [details]
MozReview Request: Bug 1268749 part 1 - Add pref to disable unprefixed fullscreen api. r=smaug

Recent regression with webcompat as a possible impact, let's uplift to Beta47 soon so we can test early and fix issues that we may find during the rest of this cycle, Beta47+
Attachment #8747614 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Part 2 of what you pushed to aurora doesn't apply to beta. Do we need another reworked patch?
I can do that again.
Flags: needinfo?(bugzilla)
This time it is conflict because of bug 1025267. More specifically, the predecessor of ENABLED_IN_CHROME flag for pseudo-classes was not implemented at all before that bug.

Could we probably uplift that to beta as well? Uplifting this patch without that gets landed sounds like a mess.
Flags: needinfo?(rkothari)
Flags: needinfo?(mats)
I don't see a problem with that.  I've asked for beta approval in bug 1025267.
Flags: needinfo?(mats)
Also, could you cc me in that bug so that I know when does that get uplifted? Or alternatively, could you comment in this bug when your patch gets uplifted there?
Flags: needinfo?(bugzilla) → needinfo?(mats)
Sure, CC:ed you on that bug.
Flags: needinfo?(mats)
Thanks.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #49)
> This time it is conflict because of bug 1025267. More specifically, the
> predecessor of ENABLED_IN_CHROME flag for pseudo-classes was not implemented
> at all before that bug.
> 
> Could we probably uplift that to beta as well? Uplifting this patch without
> that gets landed sounds like a mess.

Sure. I've approved that one for uplift to Beta.
Flags: needinfo?(rkothari)
Depends on: CVE-2016-2832
I experience various compile error when I tried to build with VS2015 locally. I wonder whether we should uplift all of those bugs to make it easier for people with VS2015 to uplift patches to Firefox 47.
Which are bug 1254963, bug 1266614, bug 1266615, and the followup commit in bug 1251225. All of them are just some trivial changes which should not hurt any behavior.
Looks like there is a new permafail after this patchset lands in beta.
Two new permafails: Mn-e10s on various platforms, and M(rc2) on Android.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #62)
> Two new permafails: Mn-e10s on various platforms, and M(rc2) on Android.

Mn-e10s is practically permafailing even before you landed these, at least on opt/pgo builds: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&fromchange=f07d961162adaf999dcaffc8c06bebfe8c5b8067&group_state=expanded&selectedJob=1076147&filter-searchStr=mn-e10s

Can't help you with the rc2 bustage, though...
ok, I guess rc2 is my fault, and that could fail when the current aurora turns to beta as well. It seems my try push in comment 8 didn't cover the android-specific tests.
The issue with Android M(rc2) is that, Element.matches(":fullscreen") throws in chrome code at https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#808

Initially I thought that we can probably check whether caller is chrome in CSS parser so that we do not return invalid for :fullscreen there. But it doesn't seem to be the right way, because nsINode has a selector cache which stores parsed selectors, which means a query from chrome may change the result for content. And checking the document's URI doesn't work for this specific case because in this cause, the target in question is in content.

So I decided to avoid querying :fullscreen inside the chrome code to fix this issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8751256 [details] [diff] [review]
followup - Avoid querying :fullscreen selector for context menu

Redirecting
Attachment #8751256 - Flags: review?(mark.finkle) → review?(margaret.leibovic)
Comment on attachment 8751256 [details] [diff] [review]
followup - Avoid querying :fullscreen selector for context menu

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

You can land this if you leave the SelectorContext bit in, but if you make any of the other suggested changes, I'm happy to re-review.

::: mobile/android/chrome/content/browser.js
@@ -2318,5 @@
> -            return aElt.matches(aSelector);
> -          return false;
> -        }
> -      };
> -    },

This API is used by a number of add-ons:
https://mxr.mozilla.org/addons/search?string=.selectorcontext%28&find=&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=addons
https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/NativeWindow/contextmenus

So you should leave this. But you can add a comment explaining that it is necessary for add-ons, so that nobody else tries to remove it in the future.

@@ +2483,5 @@
> +          }
> +          return false;
> +        }
> +      };
> +    },

You should update the documentation here to include this new context type:
https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/NativeWindow/contextmenus/add

You should also add documentation in code comments here to explain what values are supported for `aMode`, since it looks like this will always return false if a mode isn't specified.

Why not just default to returning true for any video that is matched, if a mode isn't specified, rather than requiring this "any" parameter?
Attachment #8751256 - Flags: review?(margaret.leibovic) → review+

        Attachment #8751256 -
        Attachment is obsolete: true

        Attachment #8751532 -
        Flags: review?(margaret.leibovic)

    
    

    
  
(In reply to :Margaret Leibovic from comment #70)
> You should update the documentation here to include this new context type:
> https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/
> NativeWindow/contextmenus/add

I'll update this document once it lands. Add ni? myself for this.
Flags: needinfo?(bugzilla)

    
    

    
  
Comment on attachment 8751532 [details] [diff] [review]
followup - Avoid querying :fullscreen selector for context menu

Carry your r+. I'm assuming you are fine with the changes you suggested. If you think there is any problem in this patch, feel free to ping me.
Flags: needinfo?(bugzilla) → needinfo?(margaret.leibovic)

        Attachment #8751532 -
        Flags: review?(margaret.leibovic) → review+

    
    

    
  
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa6df3f85dca80d04ca10c25f7f5f4743880b2b
Bug 1268749 followup - Avoid querying :fullscreen selector for context menu. r=margaret

    
    

    
  
I don't think I canceled my own ni? for comment 72...
Flags: needinfo?(bugzilla)

    
    

    
  
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #73)
> Comment on attachment 8751532 [details] [diff] [review]
> followup - Avoid querying :fullscreen selector for context menu
> 
> Carry your r+. I'm assuming you are fine with the changes you suggested. If
> you think there is any problem in this patch, feel free to ping me.

This looks good, thanks.
Flags: needinfo?(margaret.leibovic)

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/7fa6df3f85dc
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED

    
    

    
  
It seems there are lots of other contexts which are not documented in that page... Added the newly-added "videoContext" there, but I suppose others also need to be documented, so ni? Margaret again.
Flags: needinfo?(bugzilla) → needinfo?(margaret.leibovic)

    
    

    
  
A few patches on beta channel was marked incorrectly to bug 1268750.

https://bugzilla.mozilla.org/show_bug.cgi?id=1268750#c4
Depends on: 1273468

    
    

    
  
Oops, I forgot to correct the bug number when reland... sorry

    
    

    
  
Errr... I didn't noticed the depends on item added...

    
    

    
  
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #81)
> It seems there are lots of other contexts which are not documented in that
> page... Added the newly-added "videoContext" there, but I suppose others
> also need to be documented, so ni? Margaret again.

Any improved documentation is a win. I don't have a strong opinion here.

The future for add-ons should really be web extensions, so if something isn't supported in web extensions, I'm not too keen on adding "official" support for it.
Flags: needinfo?(margaret.leibovic)

    
    

    
  
(In reply to Jean-Yves Perrier [:teoli] from comment #86)
> Updated:
> 
> https://developer.mozilla.org/en-US/Firefox/Releases/47#Others


https://developer.mozilla.org/en-US/Firefox/Releases/47#InterfacesAPIsDOM

"The DOM fullscreen API has been unprefixed" needs an update too
Keywords: dev-doc-complete → dev-doc-needed

    
    

    
  
Good catch, I removed the sentence as it is a dup of the other comment. Thanks
Keywords: dev-doc-needed → dev-doc-complete

    
  
Depends on: 1280545
Depends on: 1285418

    
    

    
  
I've added the compatibility info to https://developer.mozilla.org/en-US/docs/Web/CSS/:fullscreen.

Sebastian
Component: DOM → DOM: Core & HTML

          You need to log in
          before you can comment on or make changes to this bug.