Fixes #217

This bug had two issues: 1. The chemistry code threw out ids on elements that were marked as `data-change="added"`. However, if they had an id already, then they had been parsed, so no need to reparse and lose the idea. 2. Moving into the base of a script that had parens passed in the contents of the parens instead of the parens. This caused the test that determines whether in base or script to failed. This fix has to be made to all languages (but doesn't involve speech).
NSoiffer · Dec 20, 2023 · 3bf6533 · 3bf6533
1 parent 9b321c2
commit 3bf6533
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 12 deletions.
diff --git a/Rules/Languages/en/navigate.yaml b/Rules/Languages/en/navigate.yaml
@@ -317,9 +317,9 @@
     - else_if: "*[1][self::m:mrow and IsBracketed(., '(', ')', false) or IsBracketed(., '[', ']', false)]" # auto zoom
       then:
       - with:
-          variables: [Move2D: "'in'", Child2D: "*[1]/*[2]"]   # phrase('in' the denominator)
+          variables: [Move2D: "'in'", Child2D: "*[1]"]   # phrase('in' the denominator)
           replace: [x: "."]
-      - set_variables: [NavNode: "*[1]/*[2]/@id"] # skip mtd
+      - set_variables: [NavNode: "*[1]/*[2]/@id"]        # skip parens/brackets
       else:
       - with:
           variables: [Move2D: "'in'", Child2D: "*[1]"]        # phrase('in' the denominator)

diff --git a/Rules/Languages/es/navigate.yaml b/Rules/Languages/es/navigate.yaml
@@ -312,9 +312,9 @@
     - else_if: "*[1][self::m:mrow and IsBracketed(., '(', ')', false) or IsBracketed(., '[', ']', false)]" # auto zoom
       then:
       - with:
-          variables: [Move2D: "'en'", Child2D: "*[1]/*[2]"]
+          variables: [Move2D: "'en'", Child2D: "*[1]"]
           replace: [x: "."]
-      - set_variables: [NavNode: "*[1]/*[2]/@id"] # skip mtd
+      - set_variables: [NavNode: "*[1]/*[2]/@id"]        # skip parens/brackets
       else:
       - with:
           variables: [Move2D: "'en'", Child2D: "*[1]"]

diff --git a/Rules/Languages/id/navigate.yaml b/Rules/Languages/id/navigate.yaml
@@ -312,9 +312,9 @@
     - else_if: "*[1][self::m:mrow and IsBracketed(., '(', ')', false) or IsBracketed(., '[', ']', false)]" # auto zoom
       then:
       - with:
-          variables: [Move2D: "'in'", Child2D: "*[1]/*[2]"]
+          variables: [Move2D: "'in'", Child2D: "*[1]"]   # phrase('in' the denominator)
           replace: [x: "."]
-      - set_variables: [NavNode: "*[1]/*[2]/@id"] # skip mtd
+      - set_variables: [NavNode: "*[1]/*[2]/@id"]        # skip parens/brackets
       else:
       - with:
           variables: [Move2D: "'in'", Child2D: "*[1]"]

diff --git a/Rules/Languages/vi/navigate.yaml b/Rules/Languages/vi/navigate.yaml
@@ -320,9 +320,9 @@
     - else_if: "*[1][self::m:mrow and IsBracketed(., '(', ')', false) or IsBracketed(., '[', ']', false)]" # auto zoom
       then:
       - with:
-          variables: [Move2D: "'ở tại'", Child2D: "*[1]/*[2]"]
+          variables: [Move2D: "'ở tại'", Child2D: "*[1]"]
           replace: [x: "."]
-      - set_variables: [NavNode: "*[1]/*[2]/@id"] # skip mtd
+      - set_variables: [NavNode: "*[1]/*[2]/@id"]        # skip parens/brackets
       else:
       - with:
           variables: [Move2D: "'ở tại'", Child2D: "*[1]"]

diff --git a/Rules/Languages/zh/tw/navigate.yaml b/Rules/Languages/zh/tw/navigate.yaml
@@ -320,9 +320,9 @@
     - else_if: "*[1][self::m:mrow and IsBracketed(., '(', ')', false) or IsBracketed(., '[', ']', false)]" # auto zoom
       then:
       - with:
-          variables: [Move2D: "'在'", Child2D: "*[1]/*[2]"]   # phrase('in' the denominator)
+          variables: [Move2D: "'在'", Child2D: "*[1]"]   # phrase('in' the denominator)
           replace: [x: "."]
-      - set_variables: [NavNode: "*[1]/*[2]/@id"] # skip mtd
+      - set_variables: [NavNode: "*[1]/*[2]/@id"]        # skip parens/brackets
       else:
       - with:
           variables: [Move2D: "'在'", Child2D: "*[1]"]        # phrase('in' the denominator)

diff --git a/src/chemistry.rs b/src/chemistry.rs
@@ -183,7 +183,7 @@ fn clean_mrow_children_restructure_pass<'a>(old_children: &[Element<'a>]) -> Opt
                 if let Some(latex_value) = child.attribute_value("data-latex") {
                     if latex_value == r"\mathrel{\longrightleftharpoons}" {
                         child.set_attribute_value("data-unicode", "\u{1f8d2}");
-                        child.set_attribute_value(MAYBE_CHEMISTRY, &"2".to_string());    // same as is_hack_for_missing_arrows()
+                        child.set_attribute_value(MAYBE_CHEMISTRY, "2");    // same as is_hack_for_missing_arrows()
                     }
                 }               
             }
@@ -532,7 +532,8 @@ fn is_changed_after_unmarking_chemistry(mathml: Element) -> bool {
         }
         if name(&mathml) == "mrow" {
             if let Some(changed_value) = mathml.attribute_value(CHANGED_ATTR) {
-                if changed_value == ADDED_ATTR_VALUE {
+                // we added an mrow, we can remove it -- but this might be already processed which is the case if "data-id-added" is true (exists)
+                if changed_value == ADDED_ATTR_VALUE && mathml.attribute("data-id-added").is_none() {
                     // mrows get added for several reasons. One of them is to canonicalize elements like msqrt that can have 1 or more children;
                     //   those should not get removed because the re-parse doesn't add those
                     // Although they would never be added, elements with fixed number of children also shouldn't have the mrow go away

diff --git a/src/navigate.rs b/src/navigate.rs
@@ -1853,6 +1853,37 @@ mod tests {
         });
     }
 
+    #[test]
+    fn base_superscript() -> Result<()> {
+        // bug #217 -- zoom into base of parenthesized script 
+        let mathml_str = "<math display='block' id='id-0' data-id-added='true'>
+            <msup data-changed='added' id='id-1' data-id-added='true'>
+                <mrow data-changed='added' id='id-2' data-id-added='true'>
+                    <mo stretchy='false' id='id-3' data-id-added='true'>(</mo>
+                    <mrow data-changed='added' id='id-4' data-id-added='true'>
+                        <mn id='id-5' data-id-added='true'>2</mn>
+                        <mo data-changed='added' id='id-6' data-id-added='true'>&#x2062;</mo>
+                        <mi id='id-7' data-id-added='true'>x</mi>
+                    </mrow>
+                    <mo stretchy='false' id='id-8' data-id-added='true'>)</mo>
+                </mrow>
+                <mn id='id-9' data-id-added='true'>2</mn>
+            </msup>
+        </math>";
+        init_default_prefs(mathml_str, "Enhanced");
+        set_preference("SpeechStyle".to_string(), "ClearSpeak".to_string()).unwrap();
+        return MATHML_INSTANCE.with(|package_instance| {
+            let package_instance = package_instance.borrow();
+            let mathml = get_element(&*package_instance);
+            let speech = test_command("ZoomIn", mathml, "id-4");
+            // tables need to check their parent for proper speech
+            assert_eq!(speech, "zoom in; in base; 2 x");
+            let speech = test_command("MoveNext", mathml, "id-9");
+            assert_eq!(speech, "move right, in superscript; 2");
+            return Ok( () );
+        });
+    }
+
 
     #[test]
     fn basic_language_test() -> Result<()> {
@@ -1880,6 +1911,7 @@ mod tests {
         test_language("es", mathml_str);
         test_language("id", mathml_str);
         test_language("vi", mathml_str);
+        test_language("zh-tw", mathml_str);
         return Ok( () );
 
         fn test_language(lang: &str, mathml_str: &str) {