1344376 - onCreatedNavigationTarget not called when clicking a target="_blank" link

Status: VERIFIED FIXED

(WebExtensions :: Request Handling, defect, P1)

Description

[u287251]

Attachment: oncreatednavigationtarget-issue.zip

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170303110153

Steps to reproduce:

1. Unzip and load-temporary-add-on the very simple extension created to reproduce the issue.
2. Open the browser console, to be able to see what the extension is reporting
3. Open the web page http://jessehakanen.net/adblockpluspopupaddon/test.html
4. Click the 3rd entry in the list, "Regular link with 'target="_blank"' attribute"



Actual results:

Nothing was reported at the browser console.

For example, clicking the first entry in the list will cause the onCreatedNavigationTarget to output the argument sent to it by the browser. This is not happening with the 3rd entry.

Also, using the debugger confirms that the listener is not being called.


Expected results:

The onCreatedNavigationTarget listener should have been called, like it is for all other cases.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1344376 - Clicking on a link with target=_blank should fire onCreatedNavigationTarget.

Review commit: https://reviewboard.mozilla.org/r/117664/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117664/

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Bill,
you were right in Bug 1190687, after fixing this issue with the attached patch, I went to re-read one of your comments and I noticed that I misread it :-(

In the attached patch:

- the fix that ensure that the target="_blank" scenario fires the onCreatedNavigationTarget event as expected
- the related additional test cases

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8844126 [details]
Bug 1344376 - Clicking on a link with target=_blank should fire onCreatedNavigationTarget.

https://reviewboard.mozilla.org/r/117664/#review119788

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Recent push to try (all the failures looks unrelated):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed4b32fed597ec766204c6747fb00dc9aada3f87&selectedJob=82432795

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fc04ff18b6c
Clicking on a link with target=_blank should fire onCreatedNavigationTarget. r=billm

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/7fc04ff18b6c

Comment 7

[Victor Carciu]

Verified as fixed in FF 55

Comment 8

[Victor Carciu]

Attachment: verif1344376.png

Postfix screenshot

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8844126 [details]
Bug 1344376 - Clicking on a link with target=_blank should fire onCreatedNavigationTarget.

Approval Request Comment

[Feature/Bug causing the regression]:

Bug 1190687 introduced the webNavigation.onCreatedNavigationTarget event in Firefox 54, and this patch (Bug 1344376) fixes a missing scenario that is supposed to fire that event but it doesn't on the implementation currently in Firefox 54. 

[User impact if declined]:

This event is going to be used by the WebExtension version of the "ads blocking" addons (e.g. Adblock Plus and UBlock Origin), and these addons need this event to be fired on this additional scenario to work correctly.

If this uplift request is not accepted, these addons will not be able to block all the ads that have to be caught using this event on Firefox 54, or their port to webextension could be furtherly deferred to Firefox 55. 

[Is this code covered by automated tests?]:

Yes, the other scenarios that should fire the onCreatedNavigationTarget event were already tested by automated tests, 
and this patch contains the one line fix as well as the additional test cases related to it.
(also, Bug 1345722 is a test only change that should be uplifted with this one to prevent these tests to fail intermittently once the new test cases are added by this patch)

[Has the fix been verified in Nightly?]:

Yes

[Needs manual test from QE? If yes, steps to reproduce]:

Yes, it would be helpful to explicitly verify the fix once uplifted, 
the STR is:
- run Firefox build which includes this fix on a new profile
- “xpinstall.signatures.required” preference set to “false” 
- Install the test addon “Back to Source Tab” WebExtension installed (https://addons-dev.allizom.org/en-US/firefox/addon/back-to-the-source-tab-test/ )
- Open the following test page in a new tab: http://jessehakanen.net/adblockpluspopupaddon/test.html
- Open the Browser Console and clear the messages
- Click the third link ("Regular link with 'target="_blank"' attribute")
- In the Browser Console the following new log message should have been logged:

```
webNavigation.onCreatedNavigationTarget event received Object { url: ..., timeStamp: ..., sourceFrameId: 0, tabId: ..., windowId: ..., sourceTabId: ... }
```
 
[List of other uplifts needed for the feature/fix]:

- Bug 1345722 (test-only change, it splits the test file where Bug 1344376 new test cases have been added by this patch to prevent them from failing intermittently)

[Is the change risky?]:

This change should have a low risk

[Why is the change risky/not risky?]:

The fix is small (it is basically a one-line change) and its impact is restricted to the lines of code that has been added by Bug 1344376 (which introduced this event for the first time), most of the patch is composed by the new test cases (which are added to the number of tests that have been already landed as part of Bug 1344376).

[String changes made/needed]:

None

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8844126 [details]
Bug 1344376 - Clicking on a link with target=_blank should fire onCreatedNavigationTarget.

Fix some potential broken "ads blocking" addons using onCreatedNavigationTarget event and was verified. Aurora54+.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8d6bf64a18bf

Comment 12

[Victor Carciu]

Verifed as fixed on FF 54

Comment 13

[Victor Carciu]

Attachment: Postfix screenshot FF54

Comment 14

[zhangyin]

For certain reasons the bug occurs again in Firefox Developer Edition 67.0b4 (64-bit)

It can be reproduced exactly in the same way as the original bug report:

1. Unzip and load-temporary-add-on the very simple extension created to reproduce the issue.
2. Open the browser console, to be able to see what the extension is reporting
3. Open the web page http://jessehakanen.net/adblockpluspopupaddon/test.html
4. Click the 3rd entry in the list, "Regular link with 'target="_blank"' attribute"

Comment 15

[Victor Carciu]

I can confirm that the bug is reproducible again. We did a mozregression and the possible source for the issue is Bug 1503681. We added a comment in that bug and added a need info for the assignee.

Comment 16

[kranthi samala]

Is this issue fixed?

Comment 17

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

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 18

[Jim Mathies [:jimm]]

(In reply to kranthi samala from comment #16)

Is this issue fixed?
according to the resolution, yet. if you're still seeing something like it, you should file a fresh bug.