911498 - page-mod/match-pattern.js requires itself

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Magnus Henoch]

Attachment: Require sdk/util/match-pattern instead of self

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20130830004004

Steps to reproduce:

Using the Flattr Firefox addon (https://github.com/flattr/fx-flattr-addon), open its web page.


Actual results:

In Firefox 23.0.1, the Flattr logo is displayed in the address bar. This is the expected behaviour.

In Aurora 25.0a2 (2013-08-30), there is no logo displayed in the address bar. Additionally, when running the addon with the Addon SDK, the following error is displayed:

console.error: flattr: 
  Message: TypeError: require(...).MatchPattern is not a constructor
  Stack:
    @resource://firefoxaddon-at-flattr-dot-com/flattr/lib/main.js:14

The line in question is this:

    flattrPattern = new (require("sdk/page-mod/match-pattern").MatchPattern)("*.flattr.com"),

This suggests that sdk/page-mod/match-pattern.js doesn't export MatchPattern. This file is a small stub that emits a deprecation warning pointing users to sdk/util/match-pattern, and then requires itself. Presumably it meant to require the replacement module, as in the attached patch. With this patch, the addon can be loaded and works as expected.

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Magnus, thanks for this! I can merge this in, or if you'd like you can send a PR on our Github repo for the change and I'll get this merged in.

Comment 2

[Magnus Henoch]

Here it is: https://github.com/mozilla/addon-sdk/pull/1231

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Great, thank you!

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/3ecc2b5c8a6b928b9aa4ee16414663148860c625
Merge pull request #1231 from legoscia/patch-1

Fix Bug 911498 Fix circular require in page-mod/match-pattern.js, r=@jsantell

Comment 5

[Magnus Henoch]

Thanks!  Will this fix be included in Firefox 25?

Comment 6

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

Out of curiosity, is 24 affected?

Comment 7

[Magnus Henoch]

No, Firefox 24 is not affected.  It still has the pre-deprecation page-mod/match-pattern.js.

Comment 8

[Alex Keybl [:akeybl]]

Where will the uplift request be? I don't see an r+ patch on this bug.

Comment 9

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

Attachment: Patch for Firefox25

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 850367
User impact if declined: The SDK match-pattern module doesn't actually get exported if users require it from the deprecated location, which could break things for some addons.
Testing completed (on m-c, etc.): This landed on m-c almost two weeks ago, no problems have been raised about it.
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

jsantell r+'d the original pull request.

Comment 10

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

(In reply to Wes Kocher (:KWierso) from comment #9)
Asked for aurora approval, but I guess 25 will be on beta by the time this is dealt with.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 805524 [details] [diff] [review]
Patch for Firefox25

[Triage Comment]
Approving for FF25 Beta uplift now that we're past merge.

Comment 13

[Dave Townsend [:mossop]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a63b5f86bd19