Closed Bug 1175685 Opened 4 years ago Closed 4 years ago

LoadContext and LoadInfo do not match for NECKO_SAFEBROWSING_APP_ID

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

Details

Attachments

(2 files, 12 obsolete files)

3.73 KB, patch
dragana
: review+
Details | Diff | Splinter Review
10.76 KB, patch
dragana
: review+
Details | Diff | Splinter Review
Bug 1125916 checks whether LoadContext and LoadInfo match.

This is failing for NECKO_SAFEBROWSING_APP_ID
I take it the LoadContext uses NECKO_SAFEBROWSING_APP_ID, and the LoadInfo uses NO_APP_ID? Most likely we should make the LoadInfo use a NECKO_SAFEBROWSING_APP_ID. Though that's somewhat tricky since I think the loadingPrincipal is the system principal. We probably need a way to insert a custom appid here somehow :(
Chattet with Jonas on IRC this afternoon, he thinks (and I agree) we should add an attribute 'cookieJarOriginAttributes' to nsILoadInfo. What type that attribute should be, I am not sure as of know, but most likely that attribute should contain more than just an appId. Tanvi and Steven are working on a similar project, I would like to get their input as well of how that cookieJarOriginAttributes should look like.
Flags: needinfo?(tanvi)
Flags: needinfo?(senglehardt)
Bug 1179985 is adding support for originAttributes throughout the tree, and Bug 1165466 is adding support to the docShell and LoadContext. Once implemented, you can access individual attributes or just grab the stringified origin (or the stringified suffix) which will contain all of the attributes set on that principal or docShell.

From chatting with Chris, it sounds like you could wait until the bugs I mentioned land, pull the attributes from the system principal and somewhere update the appId to NECKO_SAFEBROWSING_APP_ID as Jonas mentions in Comment 1. I'm not very familiar with this area of the code however.
Flags: needinfo?(senglehardt)
Jonas, Christoph, and I discussed this yesterday.

I'll start by stating the nature of the problem, as I understand it.  We want to get rid of nsILoadContext.  We want loadInfo to be the source of truth for the information stored in loadContext instead.  nsILoadContext explicitily contains the appid.  loadInfo contains the principal of the page (loadingPrincipal) which can be used to get the appid.  But in the cases where the loadingPrincipal is systemPrincipal, there is no appid information (NO_APP_ID).  Hence, sometimes nsILoadContext has an accurate appid while loadInfo does not (ex: NECKO_SAFEBROWSING_APP_ID).

To fix this problem, we need to explicitly store the appid in loadInfo.  That way we can get the information directly instead of getting it off the principal (There are actually two important reasons why we want to do this. 1) systemPrincipal doesn't have appid information. 2) there are cases in FirefoxOS where we may want to use an appid other than the one stored in the loadingPrincipal)

The appid is used as a key for a "cookiejar" (i.e. cookies, localstorage, indexeddb, etc).  There are also other keys used for the cookiejar.  All these keys are currently being consolidated into an OriginAttributes struct[1].  We should hence add the entire OriginAttributes struct to loadInfo (instead of just the appid).  Then going forward, the OriginAttributes data stored directly on the loadInfo will override the OriginAttribute data you could retrieve from the loadingPrincipal.


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1165162
https://bugzilla.mozilla.org/show_bug.cgi?id=1179985
https://bugzilla.mozilla.org/show_bug.cgi?id=1153435
Flags: needinfo?(tanvi)
Depends on: 1179985, 1165466
I will start adding OriginAttributes to LoadInfo.
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Ok I have a patch that adds OriginAttribute to LoadInfo and also fix the error from comment #0, but I want to have a discussion about api for this new parameters.

LoadInfo is created in necko so there is no interface similar to  LoadContextInfo ( LoadContextInfo.custom(...), etc.) Am I missing something? LoadInfo was never attended to have custom parameters like LoadDontextInfo. 

For the current working version of the fix I have added functions to nsIOService interface that extend newChannel and newChannelFromURI with 2 parameters, appId and inBrowser. If OriginAtributes is extended, this does look like a useful api.
Flags: needinfo?(tanvi)
Flags: needinfo?(mozilla)
(In reply to Dragana Damjanovic [:dragana] from comment #6)
> Ok I have a patch that adds OriginAttribute to LoadInfo and also fix the
> error from comment #0, but I want to have a discussion about api for this
> new parameters.

If you already have a patch, can you attach it to this bug so we can have a look? It's always easier to discuss the API if you can look at it at the same time.

> LoadInfo is created in necko so there is no interface similar to 
> LoadContextInfo ( LoadContextInfo.custom(...), etc.) Am I missing something?
> LoadInfo was never attended to have custom parameters like LoadDontextInfo. 

LoadInfo lives in the Necko world and only provides one constructor [1] which should not be modified. If we need custom fields in the LoadInfo, then we can add setters and getters. But the initial information on the loadInfo should be frozen at creation time and it should not be possible to modify that information.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/LoadInfo.cpp#22
 
> For the current working version of the fix I have added functions to
> nsIOService interface that extend newChannel and newChannelFromURI with 2
> parameters, appId and inBrowser. If OriginAtributes is extended, this does
> look like a useful api.

Sounds reasonable from what I picture in my head, but again, it would be great to see that initial patch. We definitely also want input from Jonas and potentially input from a network peer (jduell, or mcmanus).
Flags: needinfo?(mozilla)
So to be clear, I think that we by default should grab the OriginAttributes from the loadingPrincipal (Or triggeringPrincipal if loadingPrincipal is null). But then allow code to override those OriginAttributes by setting them directly on the LoadInfo.
(In reply to Dragana Damjanovic [:dragana] from comment #6)
> For the current working version of the fix I have added functions to
> nsIOService interface that extend newChannel and newChannelFromURI with 2
> parameters, appId and inBrowser. If OriginAtributes is extended, this does
> look like a useful api.

appId and inBrowser is inside the OriginAttributes struct, so you should only need to pass that.  Or, as Jonas says in comment 8, just get the information off of the principal.

Note that OriginAttributes aren't fully supported yet - https://bugzilla.mozilla.org/show_bug.cgi?id=1179985
Flags: needinfo?(tanvi)
This is what I did just to as first draft, I did last week.
So it is not exactly what Jonas suggested.
This one fix an internal use of safeBrowsing and a test (not all of them), this is just an example.
Attachment #8664050 - Attachment is obsolete: true
Attachment #8664051 - Attachment is obsolete: true
This is the first version of the patch to help deciding about the api.
Attachment #8664235 - Flags: feedback?(mozilla)
Attachment #8664235 - Flags: feedback?(jonas)
This fix SafeBrowsing and only one tet. This is just for testing, need to extend this to fix other tests.
Comment on attachment 8664235 [details] [diff] [review]
bug_1175685_addOriginAttributeToLoadInfo.patch

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

::: netwerk/base/LoadInfo.h
@@ +56,5 @@
> +           nsIPrincipal* aTriggeringPrincipal,
> +           nsINode* aLoadingContext,
> +           nsSecurityFlags aSecurityFlags,
> +           nsContentPolicyType aContentPolicyType,
> +           const OriginAttributes& aOriginAttrs);

Rather than adding another constructor, and adding a newChannelFromURI2WithOriginAttributes, it might be fine to simply allow the LoadInfo to be constructed the normal way, but then let the OriginAttributes be modified after the channel+loadinfo has been created?
(In reply to Jonas Sicking (:sicking) from comment #14)
> Comment on attachment 8664235 [details] [diff] [review]
> bug_1175685_addOriginAttributeToLoadInfo.patch
> 
> Review of attachment 8664235 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/base/LoadInfo.h
> @@ +56,5 @@
> > +           nsIPrincipal* aTriggeringPrincipal,
> > +           nsINode* aLoadingContext,
> > +           nsSecurityFlags aSecurityFlags,
> > +           nsContentPolicyType aContentPolicyType,
> > +           const OriginAttributes& aOriginAttrs);
> 
> Rather than adding another constructor, and adding a
> newChannelFromURI2WithOriginAttributes, it might be fine to simply allow the
> LoadInfo to be constructed the normal way, but then let the OriginAttributes
> be modified after the channel+loadinfo has been created?

If we have a constructor only, there is only one point in time when this parameter can be added, that is why I did it this way. And till now LoadInfo could not be changed after it is created. But if it is acceptable to construct LoadInfo and than change OriginAttributes parameter, I will update the patch.
I would like OriginAttributes to be changable by addons, using something similar to nsIContentPolicy. So I think it would be good if the OriginAttributes are mutable after construction.
Attachment #8664235 - Attachment is obsolete: true
Attachment #8664235 - Flags: feedback?(mozilla)
Attachment #8664235 - Flags: feedback?(jonas)
Attachment #8665085 - Flags: feedback?(jonas)
Attachment #8664237 - Attachment is obsolete: true
Comment on attachment 8665085 [details] [diff] [review]
bug_1175685_addOriginAttributeToLoadInfo.patch

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

::: netwerk/base/LoadInfo.cpp
@@ +276,5 @@
> +LoadInfo::GetIsInBrowser(bool* aResult)
> +{
> +  *aResult = mOriginAttributes.mInBrowser;
> +  return NS_OK;
> +}

I don't feel strongly either way, but do we need these getters? Can't consumers just get the originAttributes and get the data off of that?

Maybe we could have a non-scripted getter which returns the raw mOriginAttributes?

::: netwerk/ipc/NeckoChannelParams.ipdlh
@@ +36,5 @@
>    uint64_t        parentOuterWindowID;
>    bool            enforceSecurity;
>    bool            initialSecurityCheckDone;
> +  uint32_t        appId;
> +  bool            inBrowser;

I think you can serialize a OriginAttributes struct directly, rather than do the individual parts
Attachment #8665085 - Flags: feedback?(jonas) → feedback+
Comment on attachment 8665085 [details] [diff] [review]
bug_1175685_addOriginAttributeToLoadInfo.patch

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

::: netwerk/base/LoadInfo.cpp
@@ +82,5 @@
>      mUpgradeInsecureRequests = aLoadingContext->OwnerDoc()->GetUpgradeInsecureRequests();
>    }
> +
> +  mOriginAttributes =
> +      BasePrincipal::Cast(aLoadingPrincipal)->OriginAttributesRef();

Nit: you could move that to the initalization list.

@@ +277,5 @@
> +{
> +  *aResult = mOriginAttributes.mInBrowser;
> +  return NS_OK;
> +}
> +

I agree with Jonas, originAttributes should be encapsulated by itself. If originattributes ever changes then we would have to make sure to keep the loadInfo in sync, which seems error prone.
Attachment #8665085 - Flags: feedback+
Comment on attachment 8665086 [details] [diff] [review]
bug_1175685_add_support_for_safebrowsing.patch

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

::: toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ +101,5 @@
>  
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> +  nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo();
> +  loadInfo->SetOriginAttributes(mozilla::OriginAttributes(NECKO_SAFEBROWSING_APP_ID, false));

Nice, looks good to me!
Attachment #8665086 - Flags: feedback+
Attachment #8665085 - Attachment is obsolete: true
Attachment #8665519 - Flags: review?(michal.novotny)
Attachment #8665519 - Flags: review?(jonas)
Attachment #8665086 - Attachment is obsolete: true
Attachment #8665520 - Flags: review?(mozilla)
Comment on attachment 8665519 [details] [diff] [review]
bug_1175685_addOriginAttributeToLoadInfo.patch

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

::: netwerk/base/LoadInfo.cpp
@@ +295,5 @@
> +  return NS_OK;
> +}
> +
> +NS_IMETHODIMP
> +LoadInfo::SetOriginAttributes(OriginAttributes aOriginAttributes)

Nit: Maybe take a const-reference here to save a few cycles copying data?
Attachment #8665519 - Flags: review?(jonas) → review+
Attachment #8665519 - Flags: review?(michal.novotny)
Comment on attachment 8665519 [details] [diff] [review]
bug_1175685_addOriginAttributeToLoadInfo.patch

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

Looks good - thanks!

::: netwerk/base/LoadInfo.cpp
@@ +296,5 @@
> +}
> +
> +NS_IMETHODIMP
> +LoadInfo::SetOriginAttributes(OriginAttributes aOriginAttributes)
> +{

Yeah, please make this a const reference, no need to copy to and from the stack.

::: netwerk/base/LoadInfo.h
@@ +88,5 @@
>    uint64_t                         mParentOuterWindowID;
>    bool                             mEnforceSecurity;
>    bool                             mInitialSecurityCheckDone;
>    nsTArray<nsCOMPtr<nsIPrincipal>> mRedirectChain;
> +  OriginAttributes                 mOriginAttributes;

Nit: please make the ordering of the members match the ordering of the constructor. In other words, please have oridinAttributes before the redirectcahin.

::: netwerk/base/nsILoadInfo.idl
@@ +348,5 @@
>    /**
> +   * OriginAttributes is used to customise nsILoadInfo, e.g. add appId that
> +   * differs from appId stored in loadingPrincipal. If OriginAttribet is not
> +   * explicitly set, its value will match the values set in loadingPrincipal.
> +   */

Nit:
Customized OriginAttributes within LoadInfo to allow overwriting of the default originAttributes from the LoadingPrincipal.
Attachment #8665519 - Flags: review+
Comment on attachment 8665520 [details] [diff] [review]
bug_1175685_add_support_for_safebrowsing.patch

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

Looks good to me.

::: netwerk/test/unit/test_cookiejars_safebrowsing.js
@@ +170,5 @@
>  
>    function checkSafeBrowsingCookie() {
> +    var channel = setupChannel(checkCookiePath,
> +                               Ci.nsIScriptSecurityManager.SAFEBROWSING_APP_ID, 
> +                               false, 

nit: trailing whitespace
Attachment #8665520 - Flags: review?(mozilla) → review+
May I ask you to review necko part. Jonas an Christoph have reviewed the rest. 

I have incorporated their comments.
Attachment #8665519 - Attachment is obsolete: true
Attachment #8666703 - Flags: review?(michal.novotny)
Attachment #8665520 - Attachment is obsolete: true
Attachment #8666706 - Flags: review+
only comment changed.
Attachment #8666703 - Attachment is obsolete: true
Attachment #8666703 - Flags: review?(michal.novotny)
Attachment #8666707 - Flags: review?(michal.novotny)
Comment on attachment 8666706 [details] [diff] [review]
bug_1175685_add_support_for_safebrowsing.patch

># HG changeset patch
># User Dragana Damjanovic <dd.mozilla@gmail.com>
># Parent  9e86a592938c57452b8c9f2395ba9ba1cce7e4fe
>Bug 1175685 - Add special appId for SAFEBROWSING_APP_ID. r=ckerschb
>
>diff --git a/netwerk/test/unit/test_cookiejars_safebrowsing.js b/netwerk/test/unit/test_cookiejars_safebrowsing.js
>--- a/netwerk/test/unit/test_cookiejars_safebrowsing.js
>+++ b/netwerk/test/unit/test_cookiejars_safebrowsing.js
>@@ -62,26 +62,29 @@ function cookieCheckHandler(metadata, re
> function safebrowsingUpdateHandler(metadata, response) {
>   var cookieName = "sb-update-cookie";
>   response.setStatusLine(metadata.httpVersion, 200, "Ok");
>   response.setHeader("set-Cookie", cookieName + "=1; Path=/", false);
>   response.setHeader("Content-Type", "text/plain");
>   response.bodyOutputStream.write("Ok", "Ok".length);
> }
> 
>-function setupChannel(path, loadContext) {
>+function setupChannel(path, appId, inBrowser, loadContext) {
>   var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
>   var channel = ios.newChannel2(URL + path,
>                                 "",
>                                 null,
>                                 null,      // aLoadingNode
>                                 Services.scriptSecurityManager.getSystemPrincipal(),
>                                 null,      // aTriggeringPrincipal
>                                 Ci.nsILoadInfo.SEC_NORMAL,
>                                 Ci.nsIContentPolicy.TYPE_OTHER);
>+  var loadInfo = channel.loadInfo;
>+  loadInfo.originAttributes = { appId: appId,
>+                                inBrowser: inBrowser};
Do we need to add userContextId here, which was added to origin attributes in https://bugzilla.mozilla.org/show_bug.cgi?id=1179557.
https://dxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.h?case=true&from=usercontextid#141

>   channel.notificationCallbacks = loadContext;
>   channel.QueryInterface(Ci.nsIHttpChannel);
>   return channel;
> }
>
Attachment #8666707 - Flags: review?(michal.novotny) → review+
(In reply to Tanvi Vyas [:tanvi] from comment #30)
> Comment on attachment 8666706 [details] [diff] [review]
> bug_1175685_add_support_for_safebrowsing.patch
> 
> ># HG changeset patch
> ># User Dragana Damjanovic <dd.mozilla@gmail.com>
> ># Parent  9e86a592938c57452b8c9f2395ba9ba1cce7e4fe
> >Bug 1175685 - Add special appId for SAFEBROWSING_APP_ID. r=ckerschb
> >
> >diff --git a/netwerk/test/unit/test_cookiejars_safebrowsing.js b/netwerk/test/unit/test_cookiejars_safebrowsing.js
> >--- a/netwerk/test/unit/test_cookiejars_safebrowsing.js
> >+++ b/netwerk/test/unit/test_cookiejars_safebrowsing.js
> >@@ -62,26 +62,29 @@ function cookieCheckHandler(metadata, re
> > function safebrowsingUpdateHandler(metadata, response) {
> >   var cookieName = "sb-update-cookie";
> >   response.setStatusLine(metadata.httpVersion, 200, "Ok");
> >   response.setHeader("set-Cookie", cookieName + "=1; Path=/", false);
> >   response.setHeader("Content-Type", "text/plain");
> >   response.bodyOutputStream.write("Ok", "Ok".length);
> > }
> > 
> >-function setupChannel(path, loadContext) {
> >+function setupChannel(path, appId, inBrowser, loadContext) {
> >   var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
> >   var channel = ios.newChannel2(URL + path,
> >                                 "",
> >                                 null,
> >                                 null,      // aLoadingNode
> >                                 Services.scriptSecurityManager.getSystemPrincipal(),
> >                                 null,      // aTriggeringPrincipal
> >                                 Ci.nsILoadInfo.SEC_NORMAL,
> >                                 Ci.nsIContentPolicy.TYPE_OTHER);
> >+  var loadInfo = channel.loadInfo;
> >+  loadInfo.originAttributes = { appId: appId,
> >+                                inBrowser: inBrowser};
> Do we need to add userContextId here, which was added to origin attributes
> in https://bugzilla.mozilla.org/show_bug.cgi?id=1179557.
> https://dxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.
> h?case=true&from=usercontextid#141
>


Ok I need some more info here, maybe we should move this to a separate bug.
So userContextId is set in nsIPrincipal only? nsILoadInfo can have a custom OriginAttributes, but by setting a custom OriginAttributes we overwrite userContextID that was set from ladingPrincipal by default value. I will open  a separate bug for this.
Do we need a possibility to set a custom userontextId to a custom OriginAttributes of nsLOadInfo that is not set in loadingPrincal? Similar to appId and privateBrowsing. Or it only needs to match he loadingPrincipal one?
Flags: needinfo?(tanvi)
(In reply to Dragana Damjanovic [:dragana] from comment #31)
> (In reply to Tanvi Vyas [:tanvi] from comment #30)
> > Comment on attachment 8666706 [details] [diff] [review]
> > bug_1175685_add_support_for_safebrowsing.patch
> >
> > >-function setupChannel(path, loadContext) {
> > >+function setupChannel(path, appId, inBrowser, loadContext) {
> > >   var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
> > >   var channel = ios.newChannel2(URL + path,
> > >                                 "",
> > >                                 null,
> > >                                 null,      // aLoadingNode
> > >                                 Services.scriptSecurityManager.getSystemPrincipal(),
> > >                                 null,      // aTriggeringPrincipal
> > >                                 Ci.nsILoadInfo.SEC_NORMAL,
> > >                                 Ci.nsIContentPolicy.TYPE_OTHER);
> > >+  var loadInfo = channel.loadInfo;
> > >+  loadInfo.originAttributes = { appId: appId,
> > >+                                inBrowser: inBrowser};
> > Do we need to add userContextId here, which was added to origin attributes
> > in https://bugzilla.mozilla.org/show_bug.cgi?id=1179557.
> > https://dxr.mozilla.org/mozilla-central/source/caps/BasePrincipal.
> > h?case=true&from=usercontextid#141
> >
> 
> 
> Ok I need some more info here, maybe we should move this to a separate bug.
> So userContextId is set in nsIPrincipal only? nsILoadInfo can have a custom
> OriginAttributes, but by setting a custom OriginAttributes we overwrite
> userContextID that was set from ladingPrincipal by default value. I will
> open  a separate bug for this.
> Do we need a possibility to set a custom userontextId to a custom
> OriginAttributes of nsLOadInfo that is not set in loadingPrincal? Similar to
> appId and privateBrowsing. Or it only needs to match he loadingPrincipal one?

The value of userContextID should be the same as the value in the OriginAttributes tied to loadingPrincipal.  Does this mean we don't need to include it here?  Will we have the same problem with systemPrincipal that we had with appID?  Will systemPrincipal contain an OriginAttributes struct that is populated with meaningful values, or will they just be defaults?

I understand we need to include appID to setupChannel so that we can override the appID associated with loadingPrincipal.  But why do we need to pass inBrowser to setupChannel?  Can we rely on the inBrowser flag set with the loadingPrincipal, or do we need to overwrite that too?

Sorry Dragana, I think I have asked more questions than I have answered here.  needinfo'ing Jonas in case he has some insight.
Flags: needinfo?(tanvi) → needinfo?(jonas)
The idea for how to use the OriginAttributes on nsILoadInfo is that someone can override individual values. Ideally this can be done in JS as something like:

channel.loadInfo.originAttributes.userContextId = 12;

Though the exact syntax isn't that important. Something like

var attrs = channel.loadInfo.getOriginAttributes();
attrs.userContextId = 12;
channel.loadInfo.getOriginAttributes(attrs);

works as well.


Generally speaking, code should not take separate arguments for appid/browserflag/userContextId. The whole point of the OriginAttributes struct is that we can add additional members without worrying about adding them to all sorts of places in the codebase.

Instead you should make functions take OriginAttributes as arguments.

I'm not quite sure what these tests are doing, so I can't off the top of my head say what you should do there. But can't you do:

channel.loadInfo.originAttributes = loadContext.originAttributes;

?

As for the nsUrlClassifierStreamUpdater.cpp changes, they look fine. I.e. it seems fine that we define that that code always uses the cookie-jar where all OriginAttributes members have their default value, except the appid which is NECKO_SAFEBROWSING_APP_ID. Ideally we should migrate that to use a userContextId=NECKO_SAFEBROWSING_APP_ID or some such, but that seems like a separate bug.

I'll be on PTO until tuesday next week, but happy to jump on irc or vidyo so we can chat more in detail if this still doesn't make sense. You should also feel free to ask bholley who has a good understanding of how OriginAttributes are intended to be used.
Flags: needinfo?(jonas)
I have change the test to do:
channel.loadInfo.originAttributes = loadContext.originAttributes;

And I will land this code.
Attachment #8666707 - Attachment is obsolete: true
Attachment #8677095 - Flags: review+
Attachment #8666706 - Attachment is obsolete: true
Attachment #8677097 - Flags: review+
Rebased.
Attachment #8677095 - Attachment is obsolete: true
Attachment #8677258 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3ff5a24c4364
https://hg.mozilla.org/mozilla-central/rev/6946d1be728f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.