Closed Bug 1073487 Opened 10 years ago Closed 10 years ago

Upgrade TeXZilla to version 0.9.8

Categories

(SeaMonkey :: Composer, defect)

defect
Not set
normal

Tracking

(seamonkey2.33 fixed)

RESOLVED FIXED
seamonkey2.33
Tracking Status
seamonkey2.33 --- fixed

People

(Reporter: fredw, Assigned: fredw)

References

Details

Attachments

(1 file, 2 obsolete files)

This latest version contains many bug fixes, especially wrong character mappings due to the W3C's unicode.xml errors and the issue with the displaystyle attribute after spec alignment in Gecko 29 (bug 838506).

https://github.com/fred-wang/TeXZilla/releases/tag/v0.9.8

The list supported commands are now very close to itex2MML and quite stable, so this will allow to fix bug 1065843.
So the only file that needs to land is TeXZilla.js right?
Flags: needinfo?(fred.wang)
Yes, it's only necessary to update TeXZilla.js

The release archive contains two files:

1) TeXZilla.js, which includes the converter and a commonJS interface (after the "Export the command-line API to commonJS programs" comment).

2) TeXZilla-min.js, which includes only the converter and is moreover minified with Google Closure compiler.

As I remember, what is currently in Seamonkey is "TeXZilla-web.js", a temporary script generated during the compilation process and includes only the converter, not the commonJS interface. It's the one file that is minified afterwards to produce 2)

I believe for Seamonkey, we can just use the optimized version (TeXZilla-min.js) as anyway we don't care about tracking the exact changes. But I guess any of the versions will work.
Flags: needinfo?(fred.wang)
Attached patch Patch (obsolete) — Splinter Review
(In reply to Frédéric Wang from comment #2)
> I believe for Seamonkey, we can just use the optimized version
> (TeXZilla-min.js) as anyway we don't care about tracking the exact changes.

Most of it is generated code anyway, right?
(In reply to neil@parkwaycc.co.uk from comment #4)
> Most of it is generated code anyway, right?

Yes, it is.
(note: I only did a quick test and it seems that I get an encoding problem with the minified version...)
Attached patch Patch V2 (obsolete) — Splinter Review
This version forces the script to be loaded as UTF8 instead of the default ASCII encoding.
Attachment #8498391 - Attachment is obsolete: true
Attachment #8499366 - Flags: review?(neil)
Blocks: 1077291
Comment on attachment 8499366 [details] [diff] [review]
Patch V2

>-            .loadSubScript("chrome://editor/content/TeXZilla.js");
>+            .loadSubScript("chrome://editor/content/TeXZilla.js", {}, "UTF-8");
Are you sure {} works here? I thought it defaults to window.
(In reply to neil@parkwaycc.co.uk from comment #8)
> Are you sure {} works here? I thought it defaults to window.

It worked when I tested. I think it does not matter here because the TeXZilla object is explicitly declared as a member of the window object to prevent Google Closure compiler from removing it in ADVANCED_OPTIMIZATIONS mode (mmh, I know realize that a couple global variables generated by the closure compiler should probably be protected, but that should not have any effect here...).
(In reply to Frédéric Wang from comment #9)
> (In reply to comment #8)
> > Are you sure {} works here? I thought it defaults to window.
> 
> It worked when I tested. I think it does not matter here because the
> TeXZilla object is explicitly declared as a member of the window object to
> prevent Google Closure compiler from removing it in ADVANCED_OPTIMIZATIONS
> mode (mmh, I know realize that a couple global variables generated by the
> closure compiler should probably be protected, but that should not have any
> effect here...).

Ah, of course the previous version didn't bother with that, which confused me.
Comment on attachment 8499366 [details] [diff] [review]
Patch V2

>+            .loadSubScript("chrome://editor/content/TeXZilla.js", {}, "UTF-8");
A comment that TeXZilla.js explicitly sets window.TeXZilla wouldn't go amiss.

>+"â´","OPP";case 874:return a.a="â³","OPP";case 875:return a.a="â²","OPP";case 876:return"HIGH_SURROGATE";case 877:return"LOW_SURROGATE";case 878:return"BMP_CHARACTER"}},rules:[/^(?:.)/,/^(?:\$\$|\\\[|\$|\\\()/,/^(?:$)/,/^(?:\\[$\\])/,/^(?:[<&>])/,/^(?:[^])/,/^(?:\s*\[)/,/^(?:.)/,/^(?:([^\\\]]|(\\[\\\]]))+)/,/^(?:\])/,/^(?:\s*\{)/,/^(?:([^\\\}]|(\\[\\\}]))+)/,/^(?:\})/,/^(?:\])/,/^(?:\s+)/,/^(?:\$\$|\\\]|\$|\\\))/,/^(?:\{)/,/^(?:\})/,/^(?:\^)/,/^(?:_)/,/^(?:\.)/,/^(?:&)/,/^(?:\\\\)/,/^(?:[0-9]+(?:\.[0-9]+)?|[\u0660-\u0669]+(?:\u066B[\u0660-\u0669]+)?|(?:\uD835[\uDFCE-\uDFD7])+|(?:\uD835[\uDFCE-\uDFD7])+|(?:\uD835[\uDFD8-\uDFE1])+|(?:\uD835[\uDFE2-\uDFEB])+|(?:\uD835[\uDFEC-\uDFF5])+|(?:\uD835[\uDFF6-\uDFFF])+)/,
I happened to notice that the regexps still contain \u escapes, should we switch those to actual Unicode characters too? (No need for fresh review in that case.)
Attachment #8499366 - Flags: review?(neil) → review+
Attached patch Patch V3 r=NeilSplinter Review
Attachment #8499366 - Attachment is obsolete: true
(In reply to Frédéric Wang (:fredw) from comment #9)
(mmh, I now realize that a couple global variables generated by the
> closure compiler should probably be protected, but that should not have any
> effect here...).

I opened https://github.com/fred-wang/TeXZilla/issues/40 for that (and as a consequence of this issue, it's probably better to use "{}" than "window" for the context).

(In reply to neil@parkwaycc.co.uk from comment #11)
> A comment that TeXZilla.js explicitly sets window.TeXZilla wouldn't go amiss.

Done

> I happened to notice that the regexps still contain \u escapes, should we
> switch those to actual Unicode characters too? (No need for fresh review in
> that case.)

The conversion from \u to Unicode characters is done by Google Closure Compiler, but apparently it does not touch regexps. As I remember, using some Unicode characters for Arabic or non-BMP characters were causing problems (confusing RTL direction in some text editors, difficulty with making a regexp for character ranges spanning non-BMP characters) so the \u syntax was the easiest.
Keywords: checkin-needed
Comment on attachment 8499971 [details] [diff] [review]
Patch V3 r=Neil

Carrying forward r+ from V2 patch
Attachment #8499971 - Attachment description: Patch V3 → Patch V3 r=Neil
Attachment #8499971 - Flags: review+
Status: NEW → ASSIGNED
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/38d48d7e7992
comm-central changeset 38d48d7e7992
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.33
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: