Closed Bug 1019021 Opened 6 years ago Closed 6 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

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
Duplicate of this bug: 1025590
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: 6 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+
Duplicate of this bug: 1029489
Duplicate of this bug: 1024108
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.