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

RESOLVED FIXED in seamonkey2.30

Status

SeaMonkey
Build Config
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: kevink9876543, Assigned: kevink9876543)

Tracking

Trunk
seamonkey2.30
x86
Linux

SeaMonkey Tracking Flags

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

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
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?

Comment 1

4 years ago
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.
(Assignee)

Comment 2

4 years ago
(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.
(Assignee)

Comment 3

4 years ago
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
(Assignee)

Comment 4

4 years ago
Created attachment 8436393 [details] [diff] [review]
Hackish fix

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.
(Assignee)

Comment 5

4 years ago
Created attachment 8436395 [details]
Diff of omni.ja from a "stock" build vs. with the above patch

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.

Comment 6

4 years ago
> Source File: resource://fxmodules/WebConsoleUtils.jsm
> Line: 951
Some context around this line would be useful. Also is your addon public?

Updated

4 years ago
Summary: make package doesn't pack everything? → SeaMonkey make package doesn't package things needed for the Web Console add-on
(Assignee)

Comment 7

4 years ago
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
(Assignee)

Comment 8

4 years ago
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

Comment 9

4 years ago
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?
(Assignee)

Comment 10

4 years ago
(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".

Comment 11

4 years ago
Are for...of loops also affected?
(Assignee)

Comment 12

4 years ago
"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.)

Comment 13

4 years ago
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++
(Assignee)

Comment 14

4 years ago
Created attachment 8439440 [details] [diff] [review]
Fix v2

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

Comment 15

4 years ago
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
(Assignee)

Comment 16

4 years ago
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
Attachment #8439440 - Flags: review?(bugzilla)

Comment 17

4 years ago
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+
(Assignee)

Comment 18

4 years ago
Created 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.
Attachment #8439440 - Attachment is obsolete: true
Attachment #8442280 - Flags: review?(philip.chee)

Updated

4 years ago
Blocks: 1024108

Updated

4 years ago
Duplicate of this bug: 1025590

Comment 20

4 years ago
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+

Comment 21

4 years ago
Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/86f9d2bd2d46
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-seamonkey2.28: --- → affected
status-seamonkey2.29: --- → affected
tracking-seamonkey2.28: --- → +
tracking-seamonkey2.29: --- → +
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.30
(Assignee)

Comment 22

4 years ago
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?

Comment 23

4 years ago
> 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)
(Assignee)

Comment 24

4 years ago
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?

Comment 25

4 years ago
(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)
(Assignee)

Comment 26

4 years ago
Created attachment 8443702 [details] [diff] [review]
Minimal patch for aurora and beta?
Attachment #8443702 - Flags: review?(philip.chee)

Comment 27

4 years ago
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+

Comment 28

4 years ago
Pushed to comm-aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/fa139a736eee
Pushed to comm-beta:
https://hg.mozilla.org/releases/comm-beta/rev/6aa438f16253
status-seamonkey2.28: affected → fixed
status-seamonkey2.29: affected → fixed

Updated

4 years ago
Duplicate of this bug: 1029489

Updated

3 years ago
Duplicate of this bug: 1024108

Updated

3 years ago
No longer blocks: 1024108

Updated

3 years ago
Attachment #8442280 - Flags: approval-comm-aurora?
Blocks: 1069376
You need to log in before you can comment on or make changes to this bug.