1600275 - "Share this story" on BBC with integrated e-mail has strange behavior

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Attachment: Too many windows.mov

Affected versions

• Firefox 68.3.0esr
• Firefox 71.0 build4
• Firefox 71.0b12
• Firefox 72.0a1

Affected platforms

• macOS 10.15
• Windows 10 (x64)

Steps to reproduce

1. Open a link of choice from BBC, for example
2. Scroll down to the Share this story
3. Click on e-mail option

Expected result

• a single screen for sharing the story will be opened on macOS.
• Windows 10 integrated mail app will work as intended.

Actual result

• on macOS, there will be opened around 3 or 4 windows of the same screen to share.
• on Windows 10, the integrated mail app from windows will be unusable after selecting the e-mail from which the user will want to send the story.

Regression range

• Will see for a regression, it reproduces way back to Firefox 63.0 release.

Additional notes

• The actual issue, which is reproducible on macOS can be seen in the attachments.
• Here's an attachment with Windows 10's behavior. Sorry for the jumpy cursor, it was a glitch from the recording app.

Comment 1

[Alice0775 White]

On Windows, multiple TB compose window and/or multiple alert box(already running TB) pops up.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e9c94e412090&tochange=d25c1152bcda

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

This isn't a new regression but happens on a popular website. Mind taking a look at what went wrong, Boris?

Comment 3

[Boris Zbarsky [:bzbarsky]]

So I just tried this in Chrome, and I got two mail composition windows there too. Though I tried it a few more times and only got one composition window each those times, so that effect seems a little harder to reproduce.

In Firefox, I get three when I click the link the first time, four the second time, five the third time, six the fourth time. At that point I reloaded the page; then it was back down to three for the first click after reload.

I also tried in Safari, and I get multiple mail compose windows there, though Safari requires me to confirm allowing each one via a Safari dialog before actually talking to the OS.

What happens here is that when I click that mail icon under "Share this story", the site tries to load the mailto: URL multiple times. Each one corresponds to one mail window that opens, of course. In particular, I see three loads (after initial page load) due to the following:

1. A location.href set from page JS. In particular, this is the "Tracker:Hit:Sent:Ok" processing in https://mybbc-analytics.files.bbci.co.uk/reverb-client-js/smarttag-5.17.1.min.js on line 118, and in particular calling this function:

q = function(c) {
        var b = c;
        for (c = b.timeoutonly; b && !(b.href && 0 <= b.href.indexOf("mailto:"));) b = b.parentNode;
        if (b) {
            if (!c) a.onTrigger("Tracker:Hit:Sent:Ok", function() {
                k.timeout && clearTimeout(k.timeout);
                window.location.href = b.href
            });
            n({
                mailto: b.href,
                timeout: d
            })
        }

2. Another location.href set from a setTimeout, in https://static.bbc.co.uk/bbcdotcom/3.3.0/script/dist/bbcdotcom.js. This bottoms out in a navigateTo(e = ""mailto:?subject=Shared%20from%20BBC%20News&body=https%3A%2F%2Fwww.bbc.com%2Fnews%2Fuk-england-york-north-yorkshire-50565190"") call.

3. Another location.href set from a setTimeout, in https://nav.files.bbci.co.uk/orbit-webmodules/0.0.1-375.19a5c05/detectview/detectview.bundle.js.

So basically, this page's scripts are totally bonkers and try to load that links URL three times (or more; that code I pasted above looks to me like it would add another event listener every time it runs, which explains my observations), from event listeners attached to the link by three separate scripts. We send each such load off to the operating system mail handler.

So what do we do about it? We could try implementing the suggestion from bug 670328 and make external protocol loads subject to popup blocker controls, complete with limits on how many can happen per click. It's possible that Chrome's user activation stuff is having that effect here, basically, especially given that at least two of the loads are definitely async from the main click. The behavior there might depend on how those timeouts race with other stuff...

I don't see how the regression range from comment 1 would be relevant, offhand. Those patches just changed Document and HTMLFormElement behavior in very specific cases, and nothing in the logic flow above should really be affected by that... On the other hand, who knows what the crazy scripts on this page are doing.

Comment 4

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e9c94e412090&tochange=d25c1152bcda

It is possible that before those changes the page ends up throwing exceptions before ever getting to the various location.href sets? I can't run builds from that long ago on any machines I have on hand, apparently... :(

Comment 5

[Alice0775 White]

1. Before these patch land(i.e, on GOOD build of comment#1), Browser Console indicates the following error

• when the page is loading:

TypeError: c.prepend is not a function bbcdotcom.js:6
TypeError: c.prepend is not a function bbcdotcom.js:6
Use of getAttributeNode() is deprecated. Use getAttribute() instead. orb.min.js:1
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create gpt.js:1
A call to document.write() from an asynchronously-loaded external script was ignored. gpt.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: Object.assign is not a function frame.494af32.html:8
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
Loading mixed (insecure) display content on a secure page "http://static.bbci.co.uk/news/1.55.2536/img/news--icons-sprite.png"[Learn More] outbrain.js:17
TypeError: t.matches is not a function detectview.bundle.js:1
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create abg_lite.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create osd_listener.js:1
An unbalanced tree was written using document.write() causing data from the network to be reparsed. For more information https://developer.mozilla.org/en/Optimizing_Your_Pages_for_Speculative_Parsing container.html:8
TypeError: paragraphs[i].innerText is undefined view:10
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create index.html:29
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create sca.17.4.95.js line 31 > eval:4
a: timer started sca.17.4.95.js:32
a: 0.05ms sca.17.4.95.js:32
1575359053590	Services.HealthReport.HealthReporter	WARN	Recording new remote ID: bdba157a-4b82-46c7-b25d-2273dc7f91da
1575359054353	Services.HealthReport.HealthReporter	WARN	Marking upload as successful.
1575359054353	Services.HealthReport.HealthReporter	WARN	Removing documents from remote ID list: c9a88f17-d78d-4c61-a374-3b31951248ec
TypeError: t.matches is not a function detectview.bundle.js:1
TypeError: t.matches is not a function detectview.bundle.js:1

• And When click "Share this story":

TypeError: t.matches is not a function detectview.bundle.js:1

2. After the range(i.e, on BAD build), Browser Console indicates the following error

• when the page is loading:

TypeError: c.prepend is not a function bbcdotcom.js:6
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create gpt.js:1
A call to document.write() from an asynchronously-loaded external script was ignored. gpt.js:1
Use of getAttributeNode() is deprecated. Use getAttribute() instead. orb.min.js:1
Loading mixed (insecure) display content on a secure page "http://static.bbci.co.uk/news/1.55.2536/img/news--icons-sprite.png"[Learn More] outbrain.js:17
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create osd_listener.js:1
TypeError: paragraphs[i].innerText is undefined view:10
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create m_js_controller.js:1
An unbalanced tree was written using document.write() causing data from the network to be reparsed. For more information https://developer.mozilla.org/en/Optimizing_Your_Pages_for_Speculative_Parsing container.html:8
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create abg_lite.js:1
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create Enqz_20U.html:40
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create sca.17.4.95.js line 31 > eval:4
a: timer started sca.17.4.95.js:32
a: 0.04ms sca.17.4.95.js:32
An unbalanced tree was written using document.write() causing data from the network to be reparsed. For more information https://developer.mozilla.org/en/Optimizing_Your_Pages_for_Speculative_Parsing container.html:8
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create Enqz_20U.html:40
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create DcmEnabler_01_238.js:28
TypeError: Object.assign is not a function frame.494af32.html:8

• And When click "Share this story":

TypeError: t.innerText is undefined detectview.bundle.js:1

Comment 6

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I managed to run the nightlies from back then on Linux now, and I can't reproduce that regression range there. :( I do see a bunch of exceptions getting thrown, though, so I really do suspect that the relevant codepaths just aren't getting reached in the same way as on trunk.

Comment 7

[saral.kaushik]

Hi,

Developer at the BBC here, thanks for raising this bug. We are now aware of this and are currently looking in to it. We'll keep you updated with our progress.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:peterv, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[saral.kaushik]

Hello, this issue should now be resolved on the BBC website.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Thank you! I guess we need to decide whether there's still something we should do on the browser here too (akin to popup blocking) to prevent multiple mails being triggered by one click.

Comment 11

[Emil Ghitta, QA [:emilghitta]]

Removing the regressionwindow-wanted keyword based on Comment 1

Comment 12

[(no longer active)]

(In reply to Boris Zbarsky [:bzbarsky] from comment #10)

Thank you! I guess we need to decide whether there's still something we should do on the browser here too (akin to popup blocking) to prevent multiple mails being triggered by one click.

It appears we've already done so (see bug 566893 comment 14). So I think we can call this fixed now.

Comment 13

[Boris Zbarsky [:bzbarsky]]

It appears we've already done so

So what confuses me is that bug 167475 and bug 1517368 landed in Firefox 66.

But when I was reproducing this issue with multiple mailto windows, I was running a nightly from around 2019-12-02, which would be nightly 72 or so. But I was definitely seeing one click generate multiple mail windows.