1216937 - [B2G] On fastmail.com clicking a link in a mail does not work

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P1)

Description

[Nicolas Silva [:nical]]

I originally thought it was bug 1174532, but in this case there is nothing fiddling with touch events. :julienw looked into this and among his observations (Julien, please complete the list since you have a more precise idea of this stuff):
* the link we tried on fastmail has target="_black". if we remove this property from the debugger, clicking the link works. We tried to create a simple test case that only has the same link and could not reproduce the issue (while we have 100% repro on fastmail).
* the link does have an href, but in the system app, the code that receives the click event has the details.url property empty (actually, equal to an empty string "").

Long press on the link and clickin "open in new window" in the dialog works.

Comment 1

[Julien Wajsberg [:julienw]]

Some more information:

This is happening on fastmail.com, when clicking on a link in a mail. Unfortunately we couldn't reproduce with a smaller testcase :/

(In reply to Nicolas Silva [:nical] from comment #0)
> I originally thought it was bug 1174532, but in this case there is nothing
> fiddling with touch events. :julienw looked into this and among his
> observations (Julien, please complete the list since you have a more precise
> idea of this stuff):

> * the link we tried on fastmail has target="_black".

Actually, it's `target="_blank"` ;)

It seems to have no event handler at all (not even delegated).

> if we remove this
> property from the debugger, clicking the link works. We tried to create a
> simple test case that only has the same link and could not reproduce the
> issue (while we have 100% repro on fastmail).

See [1].
The 2 first links are using `target="_blank"` and `target="<a string>"`; the last link is exactly the same markup as in fastmail.

[1] https://everlong.org/mozilla/test-blank.html

> * the link does have an href, but in the system app, the code that receives
> the click event has the details.url property empty (actually, equal to an
> empty string "").

I added a breakpoint at [2] and I confirm that `evt.detail.url === ''`. So this seems wrong. As a result the code in [3] returns early and nothing happens.

[2] https://github.com/mozilla-b2g/gaia/blob/b6d4cf8917582b4581e91674503548113293b8b3/apps/system/js/child_window_factory.js#L48
[3] https://github.com/mozilla-b2g/gaia/blob/b6d4cf8917582b4581e91674503548113293b8b3/apps/system/js/app_window_factory.js#L110-L112


So I think the issue is somewhere in Gecko, I'd say around [4], but now it's quite far from what I know :/

[4] https://dxr.mozilla.org/mozilla-central/rev/605de27d4e7f530159842649c02075c578d7a4a5/dom/browser-element/BrowserElementParent.cpp#183

Comment 2

[Gregor Wagner [:gwagner]]

Kan-Ru, can you take a look?

Comment 3

[Gregor Wagner [:gwagner]]

Is this a regression?

Comment 4

[Nicolas Silva [:nical]]

(In reply to Gregor Wagner [:gwagner] from comment #3)
> Is this a regression?

I have had this for as long as I've used fastmail + b2g, so the bug has been around for a year or so at least.

Comment 5

[Gregor Wagner [:gwagner]]

Not a regression. Lets try to fix in 2.5 but we can't block the release on it.

Comment 6

[Julien Wajsberg [:julienw]]

IMO this is the kind of bug that's very important to address even if it's not a regression. It effectively breaks some parts of the web that work on Firefox in other platforms and in ot her browsers.

Comment 7

[Kan-Ru Chen [:kanru] (UTC+9)]

C++ stack. The window.open receives a "" url

> #0  mozilla::dom::TabChild::ProvideWindow (this=0xb1a2eb80, aParent=0xb0dcbe10, aChromeFlags=4094, aCalledFromJS=true, aPositionSpecified=false, aSizeSpecified=false, aURI=0x0, aName=..., aFeatures=..., aWindowIsNew=0xbee3f2e4, 
>     aReturn=0xbee3f31c) at /home/kanru/mozilla/gecko/dom/ipc/TabChild.cpp:1097
> #1  0xb4b62eb0 in nsWindowWatcher::OpenWindowInternal (this=this@entry=0xb1ad4bc0, aParent=aParent@entry=0xb0dcbe10, aUrl=aUrl@entry=0x0, aName=aName@entry=0xbee3f610 "_blank", aFeatures=aFeatures@entry=0x0, 
>     aCalledFromJS=aCalledFromJS@entry=true, aDialog=false, aNavigate=aNavigate@entry=true, aOpeningTab=aOpeningTab@entry=0x0, aArgv=0x0, aResult=aResult@entry=0xbee3f544)
>     at /home/kanru/mozilla/gecko/embedding/components/windowwatcher/nsWindowWatcher.cpp:663
> #2  0xb4b63842 in nsWindowWatcher::OpenWindow2 (this=0xb1ad4bc0, aParent=0xb0dcbe10, aUrl=0x0, aName=0xbee3f610 "_blank", aFeatures=0x0, aCalledFromScript=true, aDialog=false, aNavigate=true, aOpeningTab=0x0, aArguments=0x0, 
>     aResult=0xbee3f544) at /home/kanru/mozilla/gecko/embedding/components/windowwatcher/nsWindowWatcher.cpp:446
> #3  0xb4196022 in nsGlobalWindow::OpenInternal (this=this@entry=0xb0dcbe00, aUrl=..., aName=..., aOptions=..., aDialog=aDialog@entry=false, aContentModal=aContentModal@entry=false, aCalledNoScript=aCalledNoScript@entry=false, 
>     aDoJSFixups=aDoJSFixups@entry=true, aNavigate=aNavigate@entry=true, argv=argv@entry=0x0, aExtraArgument=aExtraArgument@entry=0x0, aCalleePrincipal=aCalleePrincipal@entry=0xb28924d0, aJSCallerContext=0xb0de1100, 
>     aReturn=aReturn@entry=0xbee3f6c8) at ../../../../gecko/dom/base/nsGlobalWindow.cpp:12267
> #4  0xb41968da in nsGlobalWindow::OpenJS (this=this@entry=0xb0dcbe00, aUrl=..., aName=..., aOptions=..., _retval=0xbee3f6c8) at ../../../../gecko/dom/base/nsGlobalWindow.cpp:8427
> #5  0xb4196940 in nsGlobalWindow::OpenOuter (this=this@entry=0xb0dcbe00, aUrl=..., aName=..., aOptions=..., aError=...) at ../../../../gecko/dom/base/nsGlobalWindow.cpp:8382
> #6  0xb41969d0 in nsGlobalWindow::Open (this=this@entry=0xb045bc00, aUrl=..., aName=..., aOptions=..., aError=...) at ../../../../gecko/dom/base/nsGlobalWindow.cpp:8391
> #7  0xb44555b4 in mozilla::dom::WindowBinding::open (cx=0xb0de1100, self=0xb045bc00, args=..., obj=...) at WindowBinding.cpp:1879
> #8  0xb4436ae4 in mozilla::dom::WindowBinding::genericMethod (cx=0xb0de1100, argc=<optimized out>, vp=<optimized out>) at WindowBinding.cpp:12958
> #9  0xb510d606 in CallJSNative (args=..., native=0xb4436a05 <mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*)>, cx=0xb0de1100) at ../../../../gecko/js/src/jscntxtinlines.h:235
> #10 js::Invoke (cx=0xb0de1100, args=..., construct=<optimized out>) at /home/kanru/mozilla/gecko/js/src/vm/Interpreter.cpp:784
> #11 0xb510a874 in Interpret (cx=0xb0de1100, state=...) at /home/kanru/mozilla/gecko/js/src/vm/Interpreter.cpp:3107
> #12 0xb510d370 in js::RunScript (cx=cx@entry=0xb0de1100, state=...) at /home/kanru/mozilla/gecko/js/src/vm/Interpreter.cpp:725
> #13 0xb510d65a in js::Invoke (cx=cx@entry=0xb0de1100, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/kanru/mozilla/gecko/js/src/vm/Interpreter.cpp:802
> #14 0xb510ddc4 in js::Invoke (cx=cx@entry=0xb0de1100, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xbee40288, rval=rval@entry=...) at /home/kanru/mozilla/gecko/js/src/vm/Interpreter.cpp:839
> #15 0xb51e4212 in js::jit::DoCallFallback (cx=0xb0de1100, frame=0xbee402e8, stub_=<optimized out>, argc=1, vp=0xbee40278, res=...) at /home/kanru/mozilla/gecko/js/src/jit/BaselineIC.cpp:9033
> #16 0xb2a9e9ec in ?? ()
> #17 0xb2a9e9ec in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)

JS stack. They hook something to the link.

> 0 e.views.mainWindow<.interceptLinks<(t = [object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":4]
>     this = [object Object]
> 1 t.EventTarget.fire(i = "click", r = [object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 2 i.handleEvent<(t = [object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":4]
>     this = [object Object]
> 3 r.invoke(t = [function], e = [object Object], i = [object Arguments]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 4 .invokeInRunLoop/<([object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 5 t.Tap<.end(n = [object TouchEvent]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":5]
>     this = [object Object]
> 6 e<.fire(t = "end", e = [object TouchEvent]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":5]
>     this = [object Object]
> 7 i.handleEvent<(t = [object TouchEvent], e = [object Object], i = [object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":4]
>     this = [object Object]
> 8 r.invoke(t = [function], e = [object Object], i = [object Arguments]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 9 .invokeInRunLoop/<([object TouchEvent], null, [object Object]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 10 e<.handleEvent<(e = [object TouchEvent]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":4]
>     this = [object Object]
> 11 r.invoke(t = [function], e = [object Object], i = [object Arguments]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]
> 12 .invokeInRunLoop/<([object TouchEvent]) ["https://www.fastmail.com/static/ajaxui/bootstrap-5bd37f1e.js line 1 > eval":1]
>     this = [object Object]

If I set

> FastMail.views.mainWindow.interceptLinks = t => console.log(t.target.href)

in WebIDE I get correct link in the console.

So a little peek at the .toSource(), they seem to try to window.open a "" url then document.write a <meta> refresh directive to load the url. This doesn't work for many reasons. I wonder if this should be a Evangelism bug.

Comment 8

[Gregor Wagner [:gwagner]]

Do we have the same behavior with Firefox for Android?

Comment 9

[Gregor Wagner [:gwagner]]

Or maybe nical has an adroid phone where he can try?

Comment 10

[Nicolas Silva [:nical]]

(In reply to Gregor Wagner [:gwagner] from comment #9)
> Or maybe nical has an adroid phone where he can try?

I just checked and following links in fastmail emails works perfectly on fennec.

Comment 11

[Pi Wei Cheng [:piwei] (inactive)]

QA could not reproduce this bug. I created a trial fastmail.com account, and on another account sent email with links such as 'www.google.com', 'yahoo.com' or 'http://www.cnn.com' to the fastmail account. I then signed into fastmail account via DUT, and I was able to tap on any of the links in the email and it would prompt whether I want to open the link and upon confirming I was brought to those links.

I also tried the test link provided at comment 1 ( https://everlong.org/mozilla/test-blank.html ), and I was able to open all three links via Browser on DUT.

Bug NOT reproducible on:
Device: Aries 2.5
BuildID: 20151022105033
Gaia: 29ce8ec8606e59f582375234440812b046346513
Gecko: 76bd0c01d72e64ca4f261ffdb2652a91f961e930
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Device: Flame 2.5
BuildID: 20151022030554
Gaia: 29ce8ec8606e59f582375234440812b046346513
Gecko: 76bd0c01d72e64ca4f261ffdb2652a91f961e930
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a1 (2.5) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Comment 12

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Setting the [non-reproducible] flag so contributors can find and attempt to reproduce this bug.

Comment 13

[Julien Wajsberg [:julienw]]

Thanks Kan-Ru for the hindsight, I'll try to write a simpler testcase from your findings.

Comment 14

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #7)
> This doesn't work for many reasons. I wonder if this should be a Evangelism bug.

1. BrowserElementParent::OpenWindowOOP allows empty URL but gaia rejects it as said in comment 1
2. The newly added window is a oop so the content can't use document.write

Comment 15

[Gregor Wagner [:gwagner]]

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #14)
> (In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #7)
> > This doesn't work for many reasons. I wonder if this should be a Evangelism bug.
> 
> 1. BrowserElementParent::OpenWindowOOP allows empty URL but gaia rejects it
> as said in comment 1
> 2. The newly added window is a oop so the content can't use document.write

Is this also an issue for dekstop?

Comment 16

[Julien Wajsberg [:julienw]]

Try this testcase:
https://everlong.org/mozilla/test-window-open/

The last 3 buttons work in Desktop but not in FxOS.

Basically they use `window.open('', '_blank')`. Using the exact name "_blank" triggers the code path that ignores requests with ''. I suspect that Fastmail reuses the target name from the <a> element they interfere with.

Comment 17

[Julien Wajsberg [:julienw]]

Not sure about OOP, document.write works on the 2nd and 3rd buttons in my previous testcase -- they don't use '_blank'.

Comment 18

[Julien Wajsberg [:julienw]]

Nical told me all 6 links work on Fennec.

Comment 19

[Gregor Wagner [:gwagner]]

(In reply to Julien Wajsberg [:julienw] from comment #18)
> Nical told me all 6 links work on Fennec.

Does Fennec open new pages OOP? We must handle this on electrolysis desktop somehow.

Comment 20

[Julien Wajsberg [:julienw]]

I'm sure this has nothing to do with OOP. As proof, the only change is using "_blank" or not as name. We can use document.write just fine with an opened document (2nd and 3rd button in my testcast page).

I'm also sure we should not "return" when url is "". I don't know why we do that, maybe we wanted to "return" only when url is null or undefined ? As a reminder, the faulty code is [1].

[1] https://github.com/mozilla-b2g/gaia/blob/b6d4cf8917582b4581e91674503548113293b8b3/apps/system/js/app_window_factory.js#L110-L112

I could take the bug next week if nobody can in your team. But we need to fix it IMO.

Comment 21

[GitHub Autolander Bot]

Attachment: [gaia] mcav:blank2 > mozilla-b2g:master

Comment 22

[Marcus Cavanaugh [:mcav]]

I toyed around with Julien's suggestion from Comment 20 in the attached patch. It looks like we've made some assumptions regarding blank URLs in at least a few places, meaning we'll need some additional changes other than just patching the AppWindowFactory check:

- FTU sets a default origin of "", which is used to determine if FTU is active (?!) in various places;

- task_manager_utils.js uses 'new URL(...)', which doesn't work on empty URLs (throws an exception otherwise);

- AppWindowFactory will try to reuse the same "_blank" window rather than opening a new one, since there isn't any special handling;

The attached PR patches those places, and Julien's test page from Comment 16 works with these changes. However, this warrants a more careful analysis than just this quick patch, as we haven't been very explicit about where to expect and how to handle { null, "", "_blank" } values for url/origin/name parameters in various places in the system app.

It'd be nice if someone more deeply involved in AppWindowFactory could take this and run with it, but I'm willing and able to help out if everyone's busy too.

Comment 23

[Marcus Cavanaugh [:mcav]]

Also: That patch only works sometimes. There are still some code paths left unfixed that apparently break the hackish patch here as well; probably something related to windows already being opened. So it's only a hackish start.

Comment 24

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Gregor Wagner [:gwagner] from comment #15)
> (In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #14)
> > (In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #7)
> > > This doesn't work for many reasons. I wonder if this should be a Evangelism bug.
> > 
> > 1. BrowserElementParent::OpenWindowOOP allows empty URL but gaia rejects it
> > as said in comment 1

This seems wrong. From <http://www.w3.org/TR/2009/WD-html5-20090423/browsers.html#dom-open>:

  The first argument, url, must be a valid URL for a page to load in the browsing context. If no arguments are provided, or if the first argument is the empty string, then the url argument defaults to "about:blank".

so, Gaia should allow an empty string (IIUC).

> > 2. The newly added window is a oop so the content can't use document.write

On desktop, this is fine as we create the window in the same process as the opener (same PContent). I'd be surprised if this was a problem in b2g.

Comment 25

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #24)
> > > 2. The newly added window is a oop so the content can't use document.write
> 
> On desktop, this is fine as we create the window in the same process as the
> opener (same PContent). I'd be surprised if this was a problem in b2g.

You are right. I misremembered. We also create the window in the same process in b2g.

Comment 26

[Gregor Wagner [:gwagner]]

Is this now something we can fix on the gecko side or do you think we should try gaia patch?

Comment 27

[Julien Wajsberg [:julienw]]

How could this be a fix on the gecko side given Gaia forcefully ignore empty URLs for "_blank" targets ? Or would Gecko replace `''` with `'about:blank'` ?

Comment 28

[Mahendra Potharaju [:mahe]]

Blocking with a P3, to fix and land on 2.5 branch post RA

Comment 29

[Gregor Wagner [:gwagner]]

(In reply to Mahendra Potharaju [:mahe] from comment #28)
> Blocking with a P3, to fix and land on 2.5 branch post RA

This is not a release blocker according to our blocking criteria. We will work on it post 2.5 and follow the regular uplift route. 
We either block the release for it and don't branch or we fix later and uplift based on risk vs. value analysis. We can't have both :)

Comment 30

[Julien Wajsberg [:julienw]]

(In reply to Gregor Wagner [:gwagner] from comment #29)
> (In reply to Mahendra Potharaju [:mahe] from comment #28)
> > Blocking with a P3, to fix and land on 2.5 branch post RA
> 
> This is not a release blocker according to our blocking criteria. We will
> work on it post 2.5 and follow the regular uplift route. 
> We either block the release for it and don't branch or we fix later and
> uplift based on risk vs. value analysis. We can't have both :)

I don't follow the rationale of not branching if there is still unfixed blockers. This would block the _release_, not the _branching_... We're not releasing next tuesday...

Comment 31

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Gregor Wagner [:gwagner] from comment #26)
> Is this now something we can fix on the gecko side or do you think we should
> try gaia patch?

According to the live spec[1] the algorithm is slightly different but should also use about:blank as fallback. I think it's something we should fix in gecko. This seems to also affect e10s because currently in e10s we resolve to the opener url instead of about:blank when the popup is blocked.

Comment 32

[Kan-Ru Chen [:kanru] (UTC+9)]

[1] https://html.spec.whatwg.org/multipage/browsers.html#dom-open

Comment 33

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Assign default URL and target name for window.open

Hi smaug, I'm not sure if these places are the best location to assign default values for URL and target name. You could find the reference algorithm in comment 32.

The target name seems very straightforward, just assign it "_blank" when it's nullptr. For the URL, we first have to know whether the browsing context is *new* before assign it "about:blank". The patch works for me on B2G but for desktop we still open the unblocked popup with the opener url. The open location received by ContentParent::RecvCreateWindow is always OPEN_NEWTAB, maybe that's why it behaves like this.

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8682431 [details] [diff] [review]
Assign default URL and target name for window.open

>--- a/dom/base/nsGlobalWindow.cpp
...
>   const char *options_ptr = aOptions.IsEmpty() ? nullptr : options.get();
>-  const char *name_ptr = aName.IsEmpty() ? nullptr : name.get();
>+  const char *name_ptr = aName.IsEmpty() ? "_blank" : name.get();

This looks scary. WindowWatcher has explicit code checking whether name is null.
I agree we should probably move towards this setup, given that it would follow the spec more closely, but perhaps safer would be to deal with
null name in WindowProvider or so.
In fact, it isn't clear why we need this change, given that WindowWatcher has code like 
"if (nameSpecified && !name.LowerCaseEqualsLiteral("_blank")) {"

Note, b2g has a bit wrong window.name handling anyway, https://bugzilla.mozilla.org/show_bug.cgi?id=1219998
It should move to closer to e10s setup (which may though still have some bugs)


>+  if (!aUrl) {
>+    // URL is the empty string and window is new
>+    rv = NS_NewURI(getter_AddRefs(uriToLoad), "about:blank");
>+    if (NS_FAILED(rv)) {
>+      return rv;
>+    }
>+  }
Hmm, this is also related to the change happened in https://bugzilla.mozilla.org/show_bug.cgi?id=1218594


I would prefer less risky change and deal with this on gaia side, or at least in BrowserElement, given that other Gecko users have lived with the current window.open/WindowWatcher behavior for ages
(and even for e10s we made the parameter handling to be more consistent with non-e10s case).

Comment 35

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Assign default URL for mozbrowseropenwindow event

Less risky fix and test.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8682945 [details] [diff] [review]
Assign default URL for mozbrowseropenwindow event

And we don't need to care about name as what the previous patch did?

Comment 37

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Olli Pettay [:smaug] from comment #36)
> Comment on attachment 8682945 [details] [diff] [review]
> Assign default URL for mozbrowseropenwindow event
> 
> And we don't need to care about name as what the previous patch did?

I think that will be handled in bug 1219998. But I'll check what's aName if it was passed empty string.

Comment 38

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/9887715a2176

Comment 39

[Nigel Babu [:nigelb]]

https://hg.mozilla.org/integration/b2g-inbound/rev/3f2103c31350467b0e291f1d8cfd10fc0ea9ab7d
Backed out changeset 9887715a2176 (bug 1216937) for b2g emulator bustage of its own tests

Comment 40

[Kan-Ru Chen [:kanru] (UTC+9)]

https://treeherder.mozilla.org/logviewer.html#?job_id=3265647&repo=b2g-inbound

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/578b42ed450f

Comment 42

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

https://hg.mozilla.org/mozilla-central/rev/578b42ed450f

Comment 43

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8682945 [details] [diff] [review]
Assign default URL for mozbrowseropenwindow event

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Some websites using window.open with empty URL will be unable to open a new window.
Testing completed: Landed with test case on m-c
Risk to taking this patch (and alternatives if risky): Sites previously not working will start to work. Not that risky, I think, since there are other means could exercise the same code path already.
String or UUID changes made by this patch:

Comment 44

[Mahendra Potharaju [:mahe]]

Comment on attachment 8682945 [details] [diff] [review]
Assign default URL for mozbrowseropenwindow event

Approved for 2.5 uplift. 

Thanks

Comment 45

[Kan-Ru Chen [:kanru] (UTC+9)]

for uplifting to b2g44

Comment 46

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/03858da5b716 for 2.5/b2g44