986839 - Add a LaTeX-to-MathML dialog box in Composer

Status: RESOLVED FIXED

(SeaMonkey :: Composer, enhancement)

Description

[Frédéric Wang (:fredw)]

I've been playing a bit this afternoon with the Seamonkey editor code...

I added a basic LaTeX input box that uses TeXZilla. This is similar to what Raniere implemented for MDN. That will provide at least a way to insert MathML in Composer, since bug 78128 is likely to take much more work.

That seems to work for both the composer and the mail client. Sometimes there are issues with cursor navigation/selection/edition when MathML is involved but I guess these are separate mathml/editor bugs.

Comment 1

[Frédéric Wang (:fredw)]

Attachment: Patch

Comment 2

[[e-mail]]

Comment on attachment 8395242 [details] [diff] [review]
Patch

Nice. Here's what my r- would have been for:

>+    window.openDialog("chrome://editor/content/EditorInsertMath.xul","_blank", "chrome,close,titlebar,modal,resizable", "");
(I know you copied this from insert source which is why there's a space missing after the first comma.)

>       <menuitem id="insertHTMLSource"
>                 accesskey="&insertHTMLCmd.accesskey;"
>                 command="cmd_insertHTMLWithDialog"
>                 key="inserthtmlkb"/>
>+      <menuitem id="insertMath"
>+                accesskey="&insertMathCmd.accesskey;"
>+                command="cmd_insertMathWithDialog"
>+                key="insertmathkb"/>
Sorry, I don't think this deserves its own key. (The key for insert HTML got removed in 1999 because it conflicted with something on Linux although the key attribute got forgotten. Oops!)

>diff --git a/editor/ui/dialogs/content/EditorInsertMath.html b/editor/ui/dialogs/content/EditorInsertMath.html
>new file mode 100644
Ideally I'd want to avoid HTML documents in chrome. My preference for the math preview would be:
1. Inserting the math into a XUL <description> element
2. Inserting the math into a blank document in a XUL <iframe>
3. Inserting the math into an XHTML document in a XUL <iframe>

>+        } catch(e) {
>+          /* Create an merror element with the original source. */
In the case of an error you should just disable the OK button.

>+        input.value = TeXZilla.getTeXSource(aMathSource);
There seems to be a bug here in that this will retrieve the original TeX used to create the MathML element and will ignore any edits done in Composer.

>+    <textarea id="input"
This definitely needs to be a <textbox multiline="true"> in the XUL.

>+              onkeyup="update()" onchange="update()"></textarea>
Use oninput rather than onkeyup; I can't remember whether onchange is necessary.

>+  document.documentElement.getButton("accept").removeAttribute("default");
I guess this is here because the Insert Source dialog accepts newlines in its textbox, I think that bug got fixed years ago now and the attribute can stay.

>+  gDialog.ouput = frame.contentDocument.getElementById("output");
output

>+      var newMath = gDialog.ouput.firstElementChild.cloneNode(true);
We should try to stick with importNode as per the documentation.

>\ No newline at end of file
Nit: Files should end with a newline, some editors such as vim do this automatically but others such as emacs show this as a blank line at the end.

>+  <commandset>
>+    <command id="cmd_display" oncommand="updateDisplay();"/>
>+    <command id="cmd_rtl" oncommand="updateRTL();"/>
>+  </commandset>
Don't bother with commands in dialogs, just add the code directly to the checkbox.

>+    <checkbox id="optionDisplay"
>+              label="&optionDisplay.label;" command="cmd_display"
>+              accesskey="&optionDisplay.accesskey;" checked="false"/>
>+    <checkbox id="optionRTL"
>+              label="&optionRTL.label;" command="cmd_rtl"
>+              accesskey="&optionRTL.accesskey;" checked="false"/>
Nit: checked is either "true" or absent.

>+/* parser generated by jison 0.4.13 */
Perhaps this file should live in a separate directory as it's really an external import?

>       content/editor/EdInsertTOC.xul                           (dialogs/content/EdInsertTOC.xul) 
>       content/editor/EdInsertTOC.js                            (dialogs/content/EdInsertTOC.js) 
>       content/editor/EdInsertTable.xul                         (dialogs/content/EdInsertTable.xul) 
>       content/editor/EdInsertTable.js                          (dialogs/content/EdInsertTable.js) 
>+      content/editor/EditorInsertMath.xul                      (dialogs/content/EditorInsertMath.xul) 
>+      content/editor/EditorInsertMath.js                       (dialogs/content/EditorInsertMath.js) 
>+      content/editor/EditorInsertMath.html                     (dialogs/content/EditorInsertMath.html) 
>+      content/editor/TeXZilla-web.js                           (dialogs/content/TeXZilla-web.js) 
I thought these were kept in alphabetical order but the rest of the file seems to be all over the place :-( Anyway, the Ed abbreviation appears to be slightly more consistent for these dialogs.

>+<!ENTITY mathToolbarCmd.label "Math">
>+<!ENTITY mathToolbarCmd.tooltip "Insert a MathML equation from LaTeX">
Unused?

>+<!-- LOCALIZATION NOTE (exampleText.label): DONT_TRANSLATE. This is a LaTeX example. -->
Nit: note got separated from its entity.

>+<!ENTITY optionDisplay.label "Display mode">
>+<!ENTITY optionDisplay.accesskey "D">
This makes no sense. Perhaps radiobuttons would be better, as then you could easily choose between block and inline.

Comment 3

[[e-mail]]

Attachment: Description element test

Comment 4

[[e-mail]]

Attachment: Description element test

Comment 5

[[e-mail]]

I used 1 + x + \frac{x^2}{2} + \frac{x^3}{3!} + \frac{x^4}{4!} + \frac{x^5}{5!} + \frac{x^6}{6!} + \dots as my test expression, as this overflows the preview on my PC.

Comment 6

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #2)

Thanks for the full review, Neil. I'll try to come back to that when I have time (or ask someone from the MathML team to take over the work).

> >       <menuitem id="insertHTMLSource"
> >                 accesskey="&insertHTMLCmd.accesskey;"
> >                 command="cmd_insertHTMLWithDialog"
> >                 key="inserthtmlkb"/>
> >+      <menuitem id="insertMath"
> >+                accesskey="&insertMathCmd.accesskey;"
> >+                command="cmd_insertMathWithDialog"
> >+                key="insertmathkb"/>
> Sorry, I don't think this deserves its own key. (The key for insert HTML got
> removed in 1999 because it conflicted with something on Linux although the
> key attribute got forgotten. Oops!)

OK. I just tried to copy the HTML command. BTW, key="inserthtmlkb" should be removed as it is causing assertions in debug mode now that the <key> is removed.

> >diff --git a/editor/ui/dialogs/content/EditorInsertMath.html b/editor/ui/dialogs/content/EditorInsertMath.html
> >new file mode 100644
> Ideally I'd want to avoid HTML documents in chrome. My preference for the
> math preview would be:
> 1. Inserting the math into a XUL <description> element
> 2. Inserting the math into a blank document in a XUL <iframe>
> 3. Inserting the math into an XHTML document in a XUL <iframe>

Yes, I initially wanted to to do everything in the XUL. However, TeXZilla.js is big and loading it from the XUL file takes a lot of time (but it eventually succeeds) while it is fast to load it from the XUL iframe. I'm not sure why. Any idea?

Otherwise, I can just do 

> >+/* parser generated by jison 0.4.13 */
> Perhaps this file should live in a separate directory as it's really an
> external import?

Yes, I'd like to do that, so that for example other applications based on comm-central (such as Thunderbird) could reuse the parser (actually this and the .html were the main reasons why I only request a feedback rather than a review). What would be the best place to put it?

> >       content/editor/EdInsertTOC.xul                           (dialogs/content/EdInsertTOC.xul) 
> >       content/editor/EdInsertTOC.js                            (dialogs/content/EdInsertTOC.js) 
> >       content/editor/EdInsertTable.xul                         (dialogs/content/EdInsertTable.xul) 
> >       content/editor/EdInsertTable.js                          (dialogs/content/EdInsertTable.js) 
> >+      content/editor/EditorInsertMath.xul                      (dialogs/content/EditorInsertMath.xul) 
> >+      content/editor/EditorInsertMath.js                       (dialogs/content/EditorInsertMath.js) 
> >+      content/editor/EditorInsertMath.html                     (dialogs/content/EditorInsertMath.html) 
> >+      content/editor/TeXZilla-web.js                           (dialogs/content/TeXZilla-web.js) 
> I thought these were kept in alphabetical order but the rest of the file
> seems to be all over the place :-( Anyway, the Ed abbreviation appears to be
> slightly more consistent for these dialogs.

My guess was that old dialogs use abbreviations like EdInsSrc while newer dialogs don't... But I'll change to Ed. I think there were inconsistencies with the *.dtd names for locales.

> 
> >+<!ENTITY mathToolbarCmd.label "Math">
> >+<!ENTITY mathToolbarCmd.tooltip "Insert a MathML equation from LaTeX">
> Unused?
> 

Sorry, I tried to add the button to the toolbar but gave up since this messed up the UI. Anyway, if we don't want a keyboard shortcut for math insertion, we probably don't want a button in the toolbar either. I'll remove that.

> There seems to be a bug here in that this will retrieve the original TeX used to create the MathML element and will ignore any edits done in Composer.

I'm not sure it's a bug. At the moment Composer can not really edit the MathML, but probably it should remove the annotation if it does. An alternative would be to regenerate the MathML from the TeX and compare the serialization with the current MathML to know if something changed and ignore the TeX source in that case. Not sure if it's worth doing so, though.

Comment 7

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #5)
> I used 1 + x + \frac{x^2}{2} + \frac{x^3}{3!} + \frac{x^4}{4!} +
> \frac{x^5}{5!} + \frac{x^6}{6!} + \dots as my test expression, as this
> overflows the preview on my PC.

mmmh, what do you suggest to avoid that?

Testing long expressions on a trunk build, I get a scrollbar on the iframe that I can use to read the whole preview. Also, it's possible to resize the window.

I can not see how we can do better, until we implement MathML linebreaking properly (bug 534962).

Comment 8

[Philip Chee]

>>>+/* parser generated by jison 0.4.13 */
>> Perhaps this file should live in a separate directory as it's really an
>> external import?

> Yes, I'd like to do that, so that for example other applications based on 
> comm-central (such as Thunderbird) could reuse the parser (actually this and
> the .html were the main reasons why I only request a feedback rather than a
> review). What would be the best place to put it?

Also could you add some boilerplate at the top such as THIS IS A GENERATED FILE. DO NOT EDIT THIS DIRECTLY. INSTEAD....

> +++ b/editor/ui/dialogs/content/TeXZilla-web.js
Just call this TeXZilla.js instead.

> Yes, I initially wanted to to do everything in the XUL. However, TeXZilla.js
> is big and loading it from the XUL file takes a lot of time idea?

Could you stick a profiler on it to see what's taking so long?
(https://github.com/mozilla/cleopatra)

Comment 9

[[e-mail]]

(In reply to Frédéric Wang from comment #6)
> (In reply to comment #2)
> Thanks for the full review, Neil. I'll try to come back to that when I have
> time (or ask someone from the MathML team to take over the work).
> 
> > >       <menuitem id="insertHTMLSource"
> > >                 accesskey="&insertHTMLCmd.accesskey;"
> > >                 command="cmd_insertHTMLWithDialog"
> > >                 key="inserthtmlkb"/>
> > >+      <menuitem id="insertMath"
> > >+                accesskey="&insertMathCmd.accesskey;"
> > >+                command="cmd_insertMathWithDialog"
> > >+                key="insertmathkb"/>
> > Sorry, I don't think this deserves its own key. (The key for insert HTML got
> > removed in 1999 because it conflicted with something on Linux although the
> > key attribute got forgotten. Oops!)
> 
> OK. I just tried to copy the HTML command. BTW, key="inserthtmlkb" should be
> removed as it is causing assertions in debug mode now that the <key> is
> removed.

Indeed, but you don't have to do that here if you don't want to.

> Yes, I initially wanted to to do everything in the XUL. However, TeXZilla.js
> is big and loading it from the XUL file takes a lot of time (but it
> eventually succeeds) while it is fast to load it from the XUL iframe. I'm
> not sure why. Any idea?

I didn't see a problem with my test version, which loads the script into the dialog.

> > >+/* parser generated by jison 0.4.13 */
> > Perhaps this file should live in a separate directory as it's really an
> > external import?
> 
> Yes, I'd like to do that, so that for example other applications based on
> comm-central (such as Thunderbird) could reuse the parser (actually this and
> the .html were the main reasons why I only request a feedback rather than a
> review). What would be the best place to put it?

It depends on what you mean by reusing the parser. Simply porting the editorOverlay changes should allow you to get the insert mathml dialog in Thunderbird compose for instance (it already seems to work in SeaMonkey message compose), so if somewhere under editor sounds reasonable, perhaps editor/texzilla/ or editor/ui/texzilla/content/ could work.

> My guess was that old dialogs use abbreviations like EdInsSrc while newer
> dialogs don't... But I'll change to Ed. I think there were inconsistencies
> with the *.dtd names for locales.

I think EdInsSrc is very much the odd one out here, none of the other ones abbreviate Insert.

> > There seems to be a bug here in that this will retrieve the original TeX used to create the MathML element and will ignore any edits done in Composer.
> 
> I'm not sure it's a bug. At the moment Composer can not really edit the
> MathML, but probably it should remove the annotation if it does. An
> alternative would be to regenerate the MathML from the TeX and compare the
> serialization with the current MathML to know if something changed and
> ignore the TeX source in that case. Not sure if it's worth doing so, though.

Well, if you don't want Composer to edit the MathML, then perhaps you want to set an appropriate -moz-user-select to stop someone editing it, although I think that might have to be in a core stylesheet somewhere. On a related note, enhancing EditorDblClick to edit the math might be a nice touch.

(In reply to Frédéric Wang from comment #7)
> (In reply to comment #5)
> > I used 1 + x + \frac{x^2}{2} + \frac{x^3}{3!} + \frac{x^4}{4!} +
> > \frac{x^5}{5!} + \frac{x^6}{6!} + \dots as my test expression, as this
> > overflows the preview on my PC.
> 
> mmmh, what do you suggest to avoid that?

That was really a side note that I'd thought to test that case.

> Testing long expressions on a trunk build, I get a scrollbar on the iframe
> that I can use to read the whole preview. Also, it's possible to resize the
> window.

Indeed, I had added scrollbars in my test attachment.

Comment 10

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #9)

> I didn't see a problem with my test version, which loads the script into the
> dialog.

I copied your version into the comm-central source and tried again. I still see the long delay before the window opens, so I'll need to check that. Otherwise, the edition works fine (although I would put the preview below the LaTeX input field so that there is more room for long expressions and one sees the centering in display mode).

> It depends on what you mean by reusing the parser. Simply porting the
> editorOverlay changes should allow you to get the insert mathml dialog in
> Thunderbird compose for instance (it already seems to work in SeaMonkey
> message compose), so if somewhere under editor sounds reasonable, perhaps
> editor/texzilla/ or editor/ui/texzilla/content/ could work.

Yes, I already tried in SeaMonkey message compose (comment 0) but when I built Thunderbird it didn't work, so I assumed the Thunderbird's editor was in a separate directory. I think editor/texzilla/ is fine as long as other comm-central applications have access to the files in the editor/ directory.

> Well, if you don't want Composer to edit the MathML, then perhaps you want
> to set an appropriate -moz-user-select to stop someone editing it, although
> I think that might have to be in a core stylesheet somewhere. On a related
> note, enhancing EditorDblClick to edit the math might be a nice touch.

Well, I'd like Composer to be able to edit the MathML (bug 78128), but at the moment adding or removing content inside MathML just leads to invalid markup... If it turns out to be a problem, we could handle that in a separate bug. EditorDblClick sounds a good idea, thanks.

Comment 11

[Frédéric Wang (:fredw)]

Attachment: Patch V2

This version integrates Neil's changes and addresses the comments discussed above. (for now, I'm ignoring the potential sync problem when people directly edit MathML with composer).

Loading TeXZilla.js is still very slow in the XUL for me (I haven't tried to analyze further).

I notice another issue with putting the <math> directly inside the XUL: the preview is not positioned properly.

- in display mode, the equation is not centered (so it might not be immediately clear what that mode is does, unless you use large operators with scripts)

- similarly, in inline rtl mode, the equation is not right aligned.

This and the loading issue can probably be fixed by choosing option 2) Inserting the math into a blank document in a XUL <iframe>

Comment 12

[Frédéric Wang (:fredw)]

Comment on attachment 8398481 [details] [diff] [review]
Patch V2

> Loading TeXZilla.js is still very slow in the XUL for me (I haven't tried to analyze further).

I just tried again with a release build, and that takes ~1 second to load. So there is probably something in debug mode that is slowing this down considerably.

Comment 13

[[e-mail]]

(In reply to Frédéric Wang from comment #11)
> I notice another issue with putting the <math> directly inside the XUL: the
> preview is not positioned properly.
> 
> - in display mode, the equation is not centered (so it might not be
> immediately clear what that mode is does, unless you use large operators
> with scripts)
> 
> - similarly, in inline rtl mode, the equation is not right aligned.

As I recall, centring did work in display mode for me, but I can't remember whether I tried rtl mode.

Comment 14

[[e-mail]]

Comment on attachment 8398481 [details] [diff] [review]
Patch V2

>-      goDoCommand("cmd_objectProperties");  
>+      if (element.namespaceURI === "http://www.w3.org/1998/Math/MathML") {
>+        // If the target is a MathML element, we edit its <math> ancestor.
>+        GetCurrentEditor().selection.collapse(element, 0);
>+        goDoCommand("cmd_insertMathWithDialog");
>+      } else
>+        goDoCommand("cmd_objectProperties");
This isn't the ideal idea, since there are other entries to the cmd_objectProperties command. Instead I would tweak GetObjectForProperties() and nsObjectPropertiesCommand.

>+    gDialog.mode.selectedIndex = gDialog.oldMath.getAttribute("display") == "block" ? 1 : 0;
>+    gDialog.direction.selectedIndex = gDialog.oldMath.getAttribute("dir") == "rtl" ? 1 : 0;
[I guess you can't be 100% sure that the attribute will have one of the two allowable values otherwise I would have suggested configuring value attributes on the radiobuttons.]

>+  <vbox style="width: 30em; height: 20em;">
[If you're going for vertical placement then you probably don't need the extra box, but you would have to move the default height and width to the description's parent instead.]

>+    <textbox id="input" multiline="true" oninput="updateMath()"/>
Nit: please ensure all JavaScript attributes end in a semicolon i.e. updateMath(); etc.

>+    <splitter/>
I notice that the dialog is in fact resizeable, so unless you want to be able to resize the textarea separately, you can probably do away with the splitter. (You might want to tweak the flex on the hbox and add some to the textarea though.)

>+    <hbox flex="1" style="overflow: auto;">
The centring might work if you make this a vbox instead. (Maybe my test has an error.) The right-to-left doesn't right-justify the preview, but that matches the behaviour in the document as far as I can tell.

>+      <radiogroup id="optionMode">
>+        <radio label="&optionInline.label;"
>+               accesskey="&optionInline.accesskey;"
>+               oncommand="updateMode()"/>
>+        <radio label="&optionDisplay.label;"
>+               accesskey="&optionDisplay.accesskey;"
>+               oncommand="updateMode()"/>
>+      </radiogroup>
Might as well move the oncommand to the radiogroup where the event will bubble up.

Comment 15

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #14)
> >+    gDialog.mode.selectedIndex = gDialog.oldMath.getAttribute("display") == "block" ? 1 : 0;
> >+    gDialog.direction.selectedIndex = gDialog.oldMath.getAttribute("dir") == "rtl" ? 1 : 0;
> [I guess you can't be 100% sure that the attribute will have one of the two
> allowable values otherwise I would have suggested configuring value
> attributes on the radiobuttons.]

MathML attributes with invalid values are ignored, so the behavior will be like the default "inline"/"ltr" values.

> >+    <hbox flex="1" style="overflow: auto;">
> The centring might work if you make this a vbox instead. (Maybe my test has
> an error.) The right-to-left doesn't right-justify the preview, but that
> matches the behaviour in the document as far as I can tell.

Correct. The rtl attribute actually only applies to the overall directionality of the formula and does not affect the bidi algorithm in the parent paragraph. I'll test the vbox.

Comment 16

[[e-mail]]

(In reply to Frédéric Wang from comment #15)
> (In reply to comment #14)
> > >+    gDialog.mode.selectedIndex = gDialog.oldMath.getAttribute("display") == "block" ? 1 : 0;
> > >+    gDialog.direction.selectedIndex = gDialog.oldMath.getAttribute("dir") == "rtl" ? 1 : 0;
> > [I guess you can't be 100% sure that the attribute will have one of the two
> > allowable values otherwise I would have suggested configuring value
> > attributes on the radiobuttons.]
> 
> MathML attributes with invalid values are ignored, so the behavior will be
> like the default "inline"/"ltr" values.

Actually I was referring to the behaviour of radiogroup elements when you set value attribtues on the buttons (so you can track the value of the group instead of mucking about with the selected index).

Comment 17

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #16)
> Actually I was referring to the behaviour of radiogroup elements when you
> set value attribtues on the buttons (so you can track the value of the group
> instead of mucking about with the selected index).

I think getAttribute will return unmodified invalid values, so I need to use selectedIndex to be sure to pick the right item (unless setting invalid value for the "value" attribute of the radiobuttons will fallback to the 0 index?)

Comment 18

[[e-mail]]

(In reply to Frédéric Wang comment #17)
> (In reply to comment #16)
> > Actually I was referring to the behaviour of radiogroup elements when you
> > set value attribtues on the buttons (so you can track the value of the group
> > instead of mucking about with the selected index).
> 
> I think getAttribute will return unmodified invalid values, so I need to use
> selectedIndex to be sure to pick the right item

Yes, thank you for confirming my concern.

Comment 19

[Frédéric Wang (:fredw)]

Attachment: Patch V3

Comment 20

[[e-mail]]

Comment on attachment 8400878 [details] [diff] [review]
Patch V3

>+      if (element.namespaceURI == "http://www.w3.org/1998/Math/MathML") {
>+        // If the object is a MathML element, we open the dialog to edit its
>+        // <math> ancestor.
>+        GetCurrentEditor().selection.collapse(element, 0);
>+        goDoCommand("cmd_insertMathWithDialog");
>+        return;
>+      }
>       var name = element.nodeName.toLowerCase();
>       switch (name)
In GetObjectForProperties you just check for a math node name, which seems to work, and would fit in with this switch statement.

>+function getMathAncestor(aSelection)
>+{
>+  try {
>+    // Try to select the <math> ancestor of the current selection.
>+    // XXXfredw: Should we consider more general selection?
>+    if (aSelection && aSelection.isCollapsed) {
I think you shouldn't check for a collapsed selection. In fact, getElementOrParentByTagName might be what you want. (I'll double-check.)

>+  <groupbox>
>+    <label value="&options.label;"/>
Just noticed this should probably be <caption label="&...;"/>

>+<!ENTITY options.label "Options: ">
Nit: don't put a space here.

>+<!ENTITY optionInline.label "Inline mode">
>+<!ENTITY optionInline.accesskey "I">
>+<!ENTITY optionDisplay.label "Display mode">
>+<!ENTITY optionDisplay.accesskey "D">
>+<!ENTITY optionLTR.label "Left-to-right direction">
>+<!ENTITY optionLTR.accesskey "L">
>+<!ENTITY optionRTL.label "Right-to-left direction">
>+<!ENTITY optionRTL.accesskey "R">
>+
>+<!ENTITY example.label "Example: ">
>+<!-- LOCALIZATION NOTE (exampleText.label): DONT_TRANSLATE. This is a LaTeX example. -->
>+<!ENTITY exampleText.label "\sqrt{x_1} + \frac{Ï^3}{2}">
>+
>+<!ENTITY insertButton.label "Insert">
>+<!ENTITY insertButton.accesskey "I">
Oops, inline and insert have the same access key.

Comment 21

[[e-mail]]

(In reply to comment #20)
> getElementOrParentByTagName might be what you want.

I checked and the current users are a bit inconsistent but editor.getElementOrParentByTagName("math", null) will search up from the selection's anchor node to look for the given tag.

Comment 22

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #20)
> >+      if (element.namespaceURI == "http://www.w3.org/1998/Math/MathML") {
> >+        // If the object is a MathML element, we open the dialog to edit its
> >+        // <math> ancestor.
> >+        GetCurrentEditor().selection.collapse(element, 0);
> >+        goDoCommand("cmd_insertMathWithDialog");
> >+        return;
> >+      }
> >       var name = element.nodeName.toLowerCase();
> >       switch (name)
> In GetObjectForProperties you just check for a math node name, which seems
> to work, and would fit in with this switch statement.

Strictly speaking, I should also check the namespace in GetObjectForProperties, but HTML/SVG don't have a <math> tag anyway. Here the idea was to catch all the MathML elements that could have been returned by editor.getSelectedElement("") in GetObjectForProperties (not sure when that happens). However, I can probably add a special case in GetObjectForProperties too, perhaps using getElementOrParentByTagName.

> >+function getMathAncestor(aSelection)
> >+{
> >+  try {
> >+    // Try to select the <math> ancestor of the current selection.
> >+    // XXXfredw: Should we consider more general selection?
> >+    if (aSelection && aSelection.isCollapsed) {
> I think you shouldn't check for a collapsed selection. In fact,
> getElementOrParentByTagName might be what you want. (I'll double-check.)

(In reply to [e-mail] from comment #21)
> (In reply to comment #20)
> > getElementOrParentByTagName might be what you want.
> 
> I checked and the current users are a bit inconsistent but
> editor.getElementOrParentByTagName("math", null) will search up from the
> selection's anchor node to look for the given tag.

I think I initially tried some "get parent" function of the editor, but that didn't seem to catch the <math> root, so that's why I wrote my own getMathAncestor. I'll try again.

Comment 23

[Frédéric Wang (:fredw)]

Attachment: Patch V4

So I verified again and getElementOrParentByTagName seems to work (and even better: handles non-collapsed and multi-range selection too) so I just removed the getMathAncestor function.

However my concern about editor.getSelectedElement("") in GetObjectForProperties was correct: this returns the selected child not the <math> ancestor and so it won't be handled in nsObjectPropertiesCommand. Hence I've moved the namespace check into nsObjectPropertiesCommand.

Comment 24

[Frédéric Wang (:fredw)]

Attachment: Patch V5

Just moving out TeXZilla.js from the #ifdef MOZ_SUITE...

Comment 25

[[e-mail]]

Comment on attachment 8401787 [details] [diff] [review]
Patch V5

>+    if (element.namespaceURI == "http://www.w3.org/1998/Math/MathML") {
>+      // If the object is a MathML element, we collapse the selection on it and
>+      // we return its <math> ancestor. Hence the math dialog will be used.
>+      GetCurrentEditor().selection.collapse(element, 0);
[Out of interest, how do you trigger this case? Just double-clicking some math?]

>+    <caption value="&options.label;"/>
Needs to be <caption label="&options.label;"/>. r=me with that fixed.

>+<!ENTITY options.label "Options:">
Now that I see it properly as a caption, I think the : should go too, if you don't mind.

Comment 26

[Frédéric Wang (:fredw)]

(In reply to [e-mail] from comment #25)
> Comment on attachment 8401787 [details] [diff] [review]
> Patch V5
> 
> >+    if (element.namespaceURI == "http://www.w3.org/1998/Math/MathML") {
> >+      // If the object is a MathML element, we collapse the selection on it and
> >+      // we return its <math> ancestor. Hence the math dialog will be used.
> >+      GetCurrentEditor().selection.collapse(element, 0);
> [Out of interest, how do you trigger this case? Just double-clicking some
> math?]

Yes, this happens when you double-click on some math. However, when you double-click on a text node (other than operators which are special), it selects the text instead ; so you'll have to click inside the math but outside text nodes.

Comment 27

[Frédéric Wang (:fredw)]

Attachment: Patch (final version)

Comment 28

[Frédéric Wang (:fredw)]

I'm not sure what is Seamonkey's commit rules and whether approval is required before landing a patch. I'm setting this checkin-needed, please correct me if I'm wrong.

Comment 29

[Philip Chee]

checkin-needed is correct if you need assistance landing the patch. If you have level 3 access you can check it in to comm-central yourself. As long as the tree is not CLOSED. I'm not sure if SeaMonkey being closed affects the /editor/ module.

Comment 30

[Frédéric Wang (:fredw)]

(In reply to Philip Chee from comment #29)
> checkin-needed is correct if you need assistance landing the patch. If you
> have level 3 access you can check it in to comm-central yourself. As long as
> the tree is not CLOSED. I'm not sure if SeaMonkey being closed affects the
> /editor/ module.

Thank you. I have only level 1, so I'll let someone check it in to comm-central.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/db4ed248dd29

Comment 32

[Philip Chee]

> https://hg.mozilla.org/comm-central/rev/db4ed248dd29
Yay! Thanks for all the hard work that went into this patch.

Comment 33

[Frédéric Wang (:fredw)]

(In reply to Philip Chee from comment #32)
> > https://hg.mozilla.org/comm-central/rev/db4ed248dd29
> Yay! Thanks for all the hard work that went into this patch.

No problem. Well, that's probably not the one of my contributions to Mozilla that took the most effort, but math authoring was certainly one of the oldest "feature request":

https://bugzilla.mozilla.org/show_bug.cgi?id=78128#c0
https://developer.mozilla.org/en-US/docs/Mozilla/MathML_Project/Updates/Project_Initiation_%28May_1999%29#The_Big_Picture

Comment 34

[Frédéric Wang (:fredw)]

Change documented on MDN:
https://developer.mozilla.org/en-US/docs/Web/MathML/Authoring#Input_Box

Comment 35

[Frédéric Wang (:fredw)]

Just more thoughts on this... The development version of TeXZilla contains a toImage function to generate an <img> element with MathML-in-SVG base64 content. The goal was essentially to be able to embed math equations in a <canvas> context and in particular to create 3D scientific schemas with WebGL:

http://fred-wang.github.io/TeXZilla/examples/toImage.html
http://fred-wang.github.io/TeXZilla/examples/toImageWebGL.html

I believe one could then read back the image on the canvas to generate a PNG image of the equation. So in theory, Seamonkey/Thunderbird could then provide a fallback for mail clients without MathML support or that filter MathML tags out, using only Web technologies...