Status

Other Applications
Narcissus
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Tom Austin, Assigned: Tom Austin)

Tracking

Other Branch
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

7 years ago
Currently, Narcissus does not interact with the DOM.  It needs to be updated so that it can be used within a browser.
(Assignee)

Comment 1

7 years ago
Created attachment 461304 [details] [diff] [review]
Narcissus browser support patch

This patch replaces the global object with a proxy that will reach to the underlying JS engine when needed, allowing it to use browser-specific functions when available.  This approach would allow the user to get at the underlying Narcissus constructs.  I intend to file a separate bug for that.

A number of new unit tests are passing.  A few new ones are also failing, mostly due to the 'watch' construct.
Attachment #461304 - Flags: review?(gal)

Updated

7 years ago
Component: JavaScript Engine → Narcissus
Product: Core → Other Applications
QA Contact: general → narcissus
(Assignee)

Comment 2

7 years ago
Created attachment 462627 [details] [diff] [review]
Updated patch to work with modularized Narcissus
Attachment #461304 - Attachment is obsolete: true
Attachment #462627 - Flags: review?(gal)
Attachment #461304 - Flags: review?(gal)

Comment 3

7 years ago
Comment on attachment 462627 [details] [diff] [review]
Updated patch to work with modularized Narcissus

We could wrap the global object with a proxy. What do you think?
Comment on attachment 462627 [details] [diff] [review]
Updated patch to work with modularized Narcissus

>diff --git a/js/narcissus/browser.js b/js/narcissus/browser.js
>new file mode 100644
>--- /dev/null
>+++ b/js/narcissus/browser.js

The makefile changes introduced in bug 583281 create symlinks for all files in narcissus/js*.js and the njs script depends on these. Is the Narcissus.browser module only enabled in the browser?

Dave
(Assignee)

Comment 5

7 years ago
Yup, the browser.js file is only needed for the browser.
Okay. Our file naming convention is a little goofy, but I guess it's not a big deal. Maybe just add a comment to js/src/shell/Makefile.in explaining that js*.js covers the shell-specific files and only the shell-specific files?

Dave
(Assignee)

Comment 7

7 years ago
Created attachment 463013 [details] [diff] [review]
Replacement patch for including browser support

Changes from previous patch:
*Redesigned browser.js to rely less on specific names
*Renamed browser.js to jsbrowser.js
Attachment #462627 - Attachment is obsolete: true
Attachment #463013 - Flags: review?(gal)
Attachment #462627 - Flags: review?(gal)

Comment 8

7 years ago
Comment on attachment 463013 [details] [diff] [review]
Replacement patch for including browser support

Comment style:

/*
 *
 */

We should ideally not patch the original window object.
(Assignee)

Comment 9

7 years ago
Created attachment 464230 [details] [diff] [review]
Partial patch reorganizing proxies
(Assignee)

Comment 10

7 years ago
Created attachment 464285 [details] [diff] [review]
Update patch removes need for jsbrowser.js

The new patch uses a proxy for native code functions.  As part of this, the proxy setup has been refactored.
Attachment #463013 - Attachment is obsolete: true
Attachment #464230 - Attachment is obsolete: true
Attachment #464285 - Flags: review?(gal)
Attachment #463013 - Flags: review?(gal)
(Assignee)

Comment 11

7 years ago
Comment on attachment 464285 [details] [diff] [review]
Update patch removes need for jsbrowser.js

Andreas approved with some changes:
* changed 'realGlobal' to 'hostGlobal'
* made notes of spidermonkey specific functionality
Attachment #464285 - Flags: review?(gal) → review+
(Assignee)

Comment 12

7 years ago
Changelist: http://hg.mozilla.org/tracemonkey/rev/b5abd91bc7fa
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.