Closed Bug 1019021 Opened 10 years ago Closed 10 years ago

SeaMonkey make package doesn't package things needed to run for .. in .. loops that iterate over the content window object

Categories

(SeaMonkey :: Build Config, defect)

x86
Linux
defect
Not set
normal

Tracking

(seamonkey2.28+ fixed, seamonkey2.29+ fixed)

RESOLVED FIXED
seamonkey2.30
Tracking Status
seamonkey2.28 + fixed
seamonkey2.29 + fixed

People

(Reporter: kevink9876543, Assigned: kevink9876543)

References

Details

Attachments

(2 files, 3 obsolete files)

I have an addon that ports Firefox 15's Web Console to current SeaMonkey versions.  It works fine in current Nightly builds run straight from the object-directory, but if I package that same build and try to use my addon with the packaged SeaMonkey, autocomplete of properties of the window object is broken, and the Error Console shows

Error: NS_ERROR_FACTORY_NOT_REGISTERED: 

I've been unable to figure out exactly *what* factory is not registered...
Any ideas what's going on here?
Are you the author of the addon? If not perhaps you should contact them.

Otherwise look at suite/installer/package-manifest.in and try to figure out what needs to be packaged that currently isn't.

> Error: NS_ERROR_FACTORY_NOT_REGISTERED: 
Also pasting the *whole* error message might help someone pinpoint your problem.
(In reply to Philip Chee from comment #1)
> Are you the author of the addon?
Yes, but most of the code is directly copied from Firefox 15.

> Otherwise look at suite/installer/package-manifest.in and try to figure out
> what needs to be packaged that currently isn't.
Thanks for the tip, but given such an uninformative error message, how would I know what to look for?
(Not that this is an exact regression range, but a build from c-c rev d7b54421771a / m-c rev e5f321740d10 works.)

> Also pasting the *whole* error message might help someone pinpoint your
> problem.
That *is* the whole error message.
This is the line of code that throws the error:

  for (let prop in obj) {

obj is the unwrapped Window object.  No iterations of that loop are executed.

Here's a slightly more detailed version of that error message, generated by some debug code I added to the addon, in case it's helpful:

Error: [Exception... "Factory not registered"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

function called with 0 : [object XrayWrapper [object Window]], 1 : n, 

error properties::
toString : function toString() {
    [native code]
}
message : 
result : 2147746132
name : NS_ERROR_FACTORY_NOT_REGISTERED
filename : 
lineNumber : 0
columnNumber : 0
inner : null
data : null
location : native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

Source File: resource://fxmodules/WebConsoleUtils.jsm
Line: 951
Attached patch Hackish fix (obsolete) — Splinter Review
It works for me if I make these changes, but I don't consider this patch a solution.
Basically I ran ls in the mozilla/dist/bin/components directory in the objdir, pasted in the results, replaced the existing [Components] file list with that, and deleted any duplicates I added (i.e. files that were already listed in other sections of package-manifest.in) that were causing make package to error out.
In case it's helpful for spotting what _really_ changed with that patch, here's the output of

diff -r ./omnija ../../../seamonkey/omnija

where ./omnija contains the files from the "stock" omni.ja and ../../../seamonkey/omnija contains the files from the "modified" omni.ja.
> Source File: resource://fxmodules/WebConsoleUtils.jsm
> Line: 951
Some context around this line would be useful. Also is your addon public?
Summary: make package doesn't pack everything? → SeaMonkey make package doesn't package things needed for the Web Console add-on
No, this is an addon I made for personal use.
Turns out none of that really matters though, any attempt to execute a "for (let .. in window)" loop iterating over the content window throws this error.
Summary: SeaMonkey make package doesn't package things needed for the Web Console add-on → SeaMonkey make package doesn't package things needed to run for .. in .. loops that iterate over the content window object
STR without private addons:
1) download & install NoScript latest development build from either AMO or http://noscript.net/getit#devel , restart browser
2) add to about:config, something like
noscript.surrogate.dummy.replacement : try{for (let i in window){window.content.status += "foo"}}catch(e){alert(e)}
noscript.surrogate.dummy.sources : !@^https?://
3) reload or go to a HTTP(S) website
I wasn't able to reproduce this with the zip package of SeaMonkey 2.26 release, do you have any idea as to a regression window?
(In reply to neil@parkwaycc.co.uk from comment #9)
> I wasn't able to reproduce this with the zip package of SeaMonkey 2.26
> release, do you have any idea as to a regression window?
I don't have time to find the exact changeset that caused this but see comment 2 for the revisions of the latest unaffected build I have "on hand".
Are for...of loops also affected?
"for .. of.." loops don't work on any object, and the error message is always the same:
--
[11:55:52.127] var o = {foo:"foo", bar:"baz"};for (let i of o){console.log(i)}
[11:55:52.132] TypeError: o['@@iterator'] is not a function

But they work fine on arrays:
--
[12:01:43.575] var p = ["a","c","e"];for(let i of p){console.log(i)}
[12:01:43.580] undefined
[12:01:43.580] a @ Web Console:1
[12:01:43.580] c @ Web Console:1
[12:01:43.580] e @ Web Console:1

However that is a separate issue, as even with the above patch they still don't work.  Am I doing something wrong?

(for .. of .. is like that in SeaMonkey 2.27 also.)
One thing you could do is to look at the Firefox version of package-manifest.in and see if any changesets look like it might affect the web console

http://hg.mozilla.org/mozilla-central/filelog/c482c28b35b6/browser/installer/package-manifest.in

Just a WAG but this one looks suspicious:
http://hg.mozilla.org/mozilla-central/rev/ed97de4f1d28#l4.12
Bug 965860 - Rewrite ConsoleAPI in C++
Attached patch Fix v2 (obsolete) — Splinter Review
Great suggestion, thank you!  Fixed here with this patch.
(I don't know for sure if this is a _minimal_ patch... but I think it should be up to the reviewer to decide whether that's important.)

For the purposes of requesting review, who is "mcsmurf"?
Attachment #8436393 - Attachment is obsolete: true
Attachment #8436395 - Attachment is obsolete: true
Thank you very much for the patch.

(In reply to kevink9876543 from comment #14)

> For the purposes of requesting review, who is "mcsmurf"?

Frank Wein <bugzilla@mcsmurf.de>

If you type :mcsmurf into the "requestee" box you get a autocomplete popup. Frank has several accounts. Remember not to pick the one that says "retired.

[Optional but will speed up reviews] Please give a brief description of what each change in the patch does and/or the Firefox bug number you are copying over.
Assignee: nobody → kevink9876543
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8439440 [details] [diff] [review]
Fix v2

Since mcsmurf seems otherwise occupied I'm stealing review.

(In reply to kevink9876543 from comment #16)
> Comment on attachment 8439440 [details] [diff] [review]
> Fix v2
> 
> The m-c changesets I referenced to create this patch are
> 
> https://hg.mozilla.org/mozilla-central/rev/26bac45c96b9#l3.1
> https://hg.mozilla.org/mozilla-central/rev/7c1a3e1fe6de#l2.1
> https://hg.mozilla.org/mozilla-central/rev/3ea744b43a24#l1.1
And:
http://hg.mozilla.org/mozilla-central/rev/12d670c20afc#l4.12

> +@BINPATH@/components/DataStore.manifest
> +@BINPATH@/components/DataStoreImpl.js

> +@BINPATH@/components/SlowScriptDebug.manifest
> +@BINPATH@/components/SlowScriptDebug.js

Nit: the manifest should come after the .js component.
r=me with this fixed.
Do you have check-in access to comm-central?
Attachment #8439440 - Flags: review?(bugzilla) → review+
No, I don't have checkin access so here is the patch with your comments addressed.

I also fixed the commit message.
Attachment #8439440 - Attachment is obsolete: true
Attachment #8442280 - Flags: review?(philip.chee)
Blocks: 1024108
Comment on attachment 8442280 [details] [diff] [review]
addressed review comments

> No, I don't have checkin access so here is the patch with your comments
> addressed.
> 
> I also fixed the commit message.

I'll check this in for you then. But going forward I think you should apply for commit access. You obviously know enough to fix bugs.
Attachment #8442280 - Flags: review?(philip.chee) → review+
Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/86f9d2bd2d46
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.30
Comment on attachment 8442280 [details] [diff] [review]
addressed review comments

[Approval Request Comment]
Regression caused by (bug #): could be any of the m-c changesets mentioned in comment 17
User impact if declined: any JS code containing for .. in .. loops that iterate over the content window object will fail with an obscure and uninformative error message
Testing completed (on m-c, etc.): landed on c-c
Risk to taking this patch (and alternatives if risky): Low risk - it's only adding things to packaged builds that are already present in builds run straight from the objdir
String changes made by this patch: none
Attachment #8442280 - Flags: approval-comm-aurora?
> Regression caused by (bug #): could be any of the m-c changesets mentioned
> in comment 17

Did you try one changeset at a time? Or do you need all three (I'm somewhat skeptical)
Tried comm-aurora, and it WFM adding only

@BINPATH@/components/amInstallTrigger.js

I did not try that alone on c-c as I don't have a lot of time for building atm.
Would you like me to upload that change as a separate patch?
(In reply to kevink9876543 from comment #24)
> Tried comm-aurora, and it WFM adding only
> 
> @BINPATH@/components/amInstallTrigger.js
....
> Would you like me to upload that change as a separate patch?
Yes please. Also Bug 926712 was backported to Mozilla31 == SeaMonkey 2.28 So we need this on current beta (2.28) as well as on aurora (2.29)
Attachment #8443702 - Flags: review?(philip.chee)
Comment on attachment 8443702 [details] [diff] [review]
Minimal patch for aurora and beta?

> [Approval Request Comment]
> Regression caused by (bug #): Bug 926712 (Use WebIDL to expose InstallTrigger)
> User impact if declined: any JS code containing for .. in .. loops that
> iterate over the content window object will fail with an obscure and
> uninformative error message.
> Users will not be able to access Apple iCloud https://www.icloud.com.
> Users will not be able to load the Databases list in phpMyAdmin
> Testing completed (on m-c, etc.): landed on c-c
> Risk to taking this patch (and alternatives if risky): Low risk - it's only
> adding things to packaged builds that are already present in builds run
> straight from the objdir
> String changes made by this patch: none
Attachment #8443702 - Flags: review?(philip.chee)
Attachment #8443702 - Flags: review+
Attachment #8443702 - Flags: approval-comm-beta+
Attachment #8443702 - Flags: approval-comm-aurora+
No longer blocks: 1024108
Attachment #8442280 - Flags: approval-comm-aurora?
You need to log in before you can comment on or make changes to this bug.