1088435 - [JS Error] TypeError: this.setAttribute is not a function

Status: VERIFIED FIXED

(Marketplace Graveyard :: Consumer Pages, defect, P3)

Description

[Christopher Van Wiemeersch [:cvan]]

https://rpm.newrelic.com/accounts/315282/browser/2914756/js_errors#id=1063897343&tab-js_error_drilldown_tabs_1063897343=js_error_instances_1063897343&tab-js_error_drilldown_tabs_1063915054=js_error_instances_1063915054&tab-js_error_drilldown_tabs_1063871339=js_error_instances_1063871339

––

Message         undefined is not a function
Exception type      TypeError
Approximate time    Today, 17:12
URL         /app/line/
Browser         Windows Chrome 38
User agent      Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.104 Safari/537.36

Stack trace

TypeError: undefined is not a function
    at https://marketplace.cdn.mozilla.net/media/fireplace/js/include.js?b=1413932131090:7:10936

––

Message         this.setAttribute is not a function
Exception type      TypeError
Approximate time    Today, 17:23
URL         /app/macaw
Browser         Android Android Browser 0
User agent      Mozilla/5.0 (Android; Mobile; rv:33.0) Gecko/33.0 Firefox/33.0
Stack trace
@https://marketplace.cdn.mozilla.net/media/fireplace/js/include.js?b=1413932131090:7:10931

––

Yahoo!! This found a clear, legit bug:

https://github.com/mozilla/fireplace/blob/377e84e/src/media/js/outgoing_links.js#L34-L37

`this.setAttribute` fails because in the `setTimeout` the wrong `this` is getting called. If you set up persist logs and fire up the debugger there or add a preventDefault, you'll see the same error in your console.

Here's the patch to fix:

> diff --git a/src/media/js/outgoing_links.js b/src/media/js/outgoing_links.js
> index 6a4db64..c768e44 100644
> --- a/src/media/js/outgoing_links.js
> +++ b/src/media/js/outgoing_links.js
> @@ -34,7 +34,7 @@ define('outgoing_links', ['capabilities', 'z'], function(capabilities, z) {
>          setTimeout(function() {
>              // Put back the real destination:
>              this.setAttribute('href', href);
> -        }, 100);
> +        }.bind(this), 100);
>          return true;
>      });
>  });
>

Comment 1

[Mathieu Pillard [:mat]]

Assigning to cvan since he has a patch that works (tested it locally) :)

Comment 2

[Christopher Van Wiemeersch [:cvan]]

PR: https://github.com/mozilla/fireplace/pull/750

Comment 3

[Christopher Van Wiemeersch [:cvan]]

Merged:
https://github.com/mozilla/flue/pull/23
https://github.com/mozilla/flue/commit/3a8637f7

https://github.com/mozilla/fireplace/pull/750
https://github.com/mozilla/fireplace/commit/31db2d2

––

QA:

Easiest to test on desktop…

1. Load an app detail page (e.g., https://marketplace-dev.allizom.org/app/line).
2. Click a link in the app description (e.g., LINE's "Go to download").
3. Open Firefox Developer Tools, open Toolbox Options (the gear icon), check "Enable persistent logs" (under Common Preferences).
4. Go back to the Console.
5. The link should open in a new tab. Close the tab to go back to the Marketplace.
6. You should *not* see this JS error anymore:

    > TypeError: this.setAttribute is not a function

    Since this fix hasn't shipped to production yet, you can reproduce the error by loading https://marketplace.firefox.com/app/line and repeating these steps above:
    https://www.dropbox.com/s/t6kptfxgosehtcm/Screenshot%202014-11-12%2013.37.24.png?dl=0

Comment 4

[Iulian Timis]

Verified as fixed in https://marketplace-dev.allizom.org/ on FF36 (Win 7).
Attaching postfix screencast http://screencast.com/t/K6Wzb2IvtlR
Closing bug.