Desktop: Fixes #11617: Links from imported notes from OneNote were being wrongly rendered

packages/lib/services/interop/InteropService_Importer_OneNote.test.ts

@@ -176,4 +176,35 @@ describe('InteropService_Importer_OneNote', () => {
 
 		BaseModel.setIdGenerator(originalIdGenerator);
 	});
+
+	skipIfNotCI('should remove hyperlink from title', async () => {
+		let idx = 0;
+		const originalIdGenerator = BaseModel.setIdGenerator(() => String(idx++));
+		const notes = await importNote(`${supportDir}/onenote/remove_hyperlink_on_title.zip`);
+
+		for (const note of notes) {
+			expect(note.body).toMatchSnapshot(note.title);
+		}
+		BaseModel.setIdGenerator(originalIdGenerator);
+	});
+
+	skipIfNotCI('should group link parts even if they have different css styles', async () => {
+		const notes = await importNote(`${supportDir}/onenote/remove_hyperlink_on_title.zip`);
+
+		const noteToTest = notes.find(n => n.title === 'Tips from a Pro Using Trees for Dramatic Landscape Photography');
+
+		expect(noteToTest).toBeTruthy();
+		expect(noteToTest.body.includes('<a href="onenote:https://d.docs.live.net/c8d3bbab7f1acf3a/Documents/Photography/风景.one#Tips%20from%20a%20Pro%20Using%20Trees%20for%20Dramatic%20Landscape%20Photography&section-id={262ADDFB-A4DC-4453-A239-0024D6769962}&page-id={88D803A5-4F43-48D4-9B16-4C024F5787DC}&end" style="">Tips from a Pro: Using Trees for Dramatic Landscape Photography</a>')).toBe(true);
+	});
+
+	skipIfNotCI('should render links properly by ignoring wrongly set indices when the first character is a hyperlink marker', async () => {
+		let idx = 0;
+		const originalIdGenerator = BaseModel.setIdGenerator(() => String(idx++));
+		const notes = await importNote(`${supportDir}/onenote/hyperlink_marker_as_first_character.zip`);
+
+		for (const note of notes) {
+			expect(note.body).toMatchSnapshot(note.title);
+		}
+		BaseModel.setIdGenerator(originalIdGenerator);
+	});
 });

packages/onenote-converter/src/notebook.rs

@@ -84,7 +84,7 @@ impl Renderer {
         let section_path = renderer.render(section, notebook_dir)?;
         log!("section_path: {:?}", section_path);
 
-        let path_from_base_dir = unsafe { remove_prefix(section_path.as_str(), base_dir.as_str()) }
+        let path_from_base_dir = unsafe { remove_prefix(section_path, base_dir.as_str()) }
             .unwrap()
             .as_string()
             .unwrap();

packages/onenote-converter/src/page/mod.rs

@@ -35,7 +35,7 @@ impl<'a> Renderer<'a> {
     }
 
     pub(crate) fn render_page(&mut self, page: &Page) -> Result<String> {
-        let title_text = page.title_text().unwrap_or("Untitled Page");
+        let title_text = page.title_text().unwrap_or("Untitled Page".to_string());
 
         let mut content = String::new();
 
@@ -70,7 +70,7 @@ impl<'a> Renderer<'a> {
 
         content.push_str(&page_content);
 
-        crate::templates::page::render(title_text, &content, &self.global_styles)
+        crate::templates::page::render(&title_text, &content, &self.global_styles)
     }
 
     pub(crate) fn gen_class(&mut self, prefix: &str) -> String {

packages/onenote-converter/src/page/rich_text.rs

@@ -74,11 +74,24 @@ impl<'a> Renderer<'a> {
         // all the styles to be shifted by minus one.
         // A better solution would be to look if there isn't anything wrong with the parser,
         // but I haven't found what could be causing this yet.
-        if text.starts_with("\u{000B}") && !indices.is_empty(){
+        if text.starts_with("\u{000B}") && !indices.is_empty() {
             indices.remove(0);
             styles.pop();
         }
 
+        // Probably the best solution here would be to rewrite the render_hyperlink to take this
+        // case in account, backtracking if necessary, but this will do for now
+        if text.starts_with("\u{fddf}") {
+            let first_indice = match indices.get(0) {
+                Some(i) => *i,
+                None => 0,
+            };
+            if first_indice == 1 {
+                indices.remove(0);
+                styles.pop();
+            }
+        }
+
         if indices.is_empty() {
             return Ok(fix_newlines(&text));
         }
@@ -100,19 +113,26 @@ impl<'a> Renderer<'a> {
         }
 
         let mut in_hyperlink = false;
+        let mut is_href_finished = true;
 
         let content = parts
             .into_iter()
             .rev()
             .zip(styles.iter())
             .map(|(text, style)| {
                 if style.hyperlink() {
-                    let text = self.render_hyperlink(text, style, in_hyperlink);
-                    in_hyperlink = true;
-
-                    text
+                    let result =
+                        self.render_hyperlink(text.clone(), style, in_hyperlink, is_href_finished);
+                    if result.is_ok() {
+                        in_hyperlink = true;
+                        is_href_finished = result.as_ref().unwrap().1;
+                        Ok(result.unwrap().0)
+                    } else {
+                        Ok(text)
+                    }
                 } else {
                     in_hyperlink = false;
+                    is_href_finished = true;
 
                     let style = self.parse_style(style);
 
@@ -128,30 +148,50 @@ impl<'a> Renderer<'a> {
         Ok(fix_newlines(&content))
     }
 
+    /// The hyperlink is delimited by the HYPERLINK_MARKER until the closing double quote
+    /// In some cases the hyperlink is broken in more than one style (e.g.: when there are
+    /// chinese characters on the url path), so we must keep track of the href status
+    /// https://github.com/laurent22/joplin/issues/11600
     fn render_hyperlink(
         &self,
         text: String,
         style: &ParagraphStyling,
         in_hyperlink: bool,
-    ) -> Result<String> {
+        is_href_finished: bool,
+    ) -> Result<(String, bool)> {
         const HYPERLINK_MARKER: &str = "\u{fddf}HYPERLINK \"";
 
         let style = self.parse_style(style);
 
         if text.starts_with(HYPERLINK_MARKER) {
             let url = text
                 .strip_prefix(HYPERLINK_MARKER)
-                .wrap_err("Hyperlink has no start marker")?
-                .strip_suffix('"')
-                .wrap_err("Hyperlink has no end marker")?;
-
-            Ok(format!("<a href=\"{}\" style=\"{}\">", url, style))
-        } else if in_hyperlink {
-            Ok(text + "</a>")
+                .wrap_err("Hyperlink has no start marker")?;
+
+            let url_2 = url.strip_suffix('"');
+
+            if url_2.is_some() {
+                return Ok((
+                    format!("<a href=\"{}\" style=\"{}\">", url_2.unwrap(), style),
+                    true,
+                ));
+            } else {
+                // If we didn't find the double quotes means that href still has content in following styles
+                Ok((format!("<a href=\"{}", url), false))
+            }
+        } else if in_hyperlink && is_href_finished {
+            Ok((text + "</a>", true))
+        } else if in_hyperlink && !is_href_finished {
+            let url = text.strip_suffix('"');
+            if url.is_some() {
+                return Ok((format!("{}\" style=\"{}\">", url.unwrap(), style), true));
+            } else {
+                Ok((text, false))
+            }
         } else {
-            Ok(format!(
-                "<a href=\"{}\" style=\"{}\">{}</a>",
-                text, style, text
+            Ok((
+                format!("<a href=\"{}\" style=\"{}\">{}</a>", text, style, text),
+                true,
             ))
         }
     }

packages/onenote-converter/src/parser/one/property_set/page_metadata.rs

@@ -32,10 +32,13 @@ pub(crate) fn parse(object: &Object) -> Result<Data> {
 
     let entity_guid = simple::parse_guid(PropertyType::NotebookManagementEntityGuid, object)?
         .ok_or_else(|| ErrorKind::MalformedOneNoteFileData("page metadata has no guid".into()))?;
-    let cached_title =
-        simple::parse_string(PropertyType::CachedTitleString, object)?.ok_or_else(|| {
-            ErrorKind::MalformedOneNoteFileData("page metadata has no cached title".into())
-        })?;
+    // The page might not have a title but we can use the first Section outline from the body as the fallback later
+    let cached_title = simple::parse_string(PropertyType::CachedTitleString, object)?
+        .ok_or_else(|| {
+            let guid = simple::parse_guid(PropertyType::NotebookManagementEntityGuid, object);
+            return guid.map(|g| g.unwrap().to_string());
+        })
+        .unwrap_or("Untitled Page".to_string());
     let schema_revision_in_order_to_read =
         simple::parse_u32(PropertyType::SchemaRevisionInOrderToRead, object)?;
     let schema_revision_in_order_to_write =

packages/onenote-converter/src/parser/onenote/page.rs

@@ -62,16 +62,23 @@ impl Page {
     /// The page's title text.
     ///
     /// This is calculated using a heuristic similar to the one OneNote uses.
-    pub fn title_text(&self) -> Option<&str> {
+    pub fn title_text(&self) -> Option<String> {
         self.title
             .as_ref()
             .and_then(|title| title.contents.first())
             .and_then(Self::outline_text)
+            .and_then(|t| Some(Self::remove_hyperlink(t.to_owned())))
             .or_else(|| {
                 self.contents
                     .iter()
                     .filter_map(|page_content| page_content.outline())
-                    .filter_map(Self::outline_text)
+                    .filter_map(|t| {
+                        let v = Self::outline_text(t);
+                        if v.is_none() {
+                            return None;
+                        }
+                        return Some(Self::remove_hyperlink(v.unwrap().to_owned()));
+                    })
                     .next()
             })
     }
@@ -85,6 +92,33 @@ impl Page {
             .and_then(|content| content.rich_text())
             .and_then(|text| Some(&*text.text).filter(|s| !s.is_empty()))
     }
+
+    fn remove_hyperlink(title: String) -> String {
+        const HYPERLINK_MARKER: &str = "\u{fddf}HYPERLINK \"";
+
+        let mut title_copy = title.clone();
+
+        loop {
+            // Find the first hyperlink mark
+            if let Some(marker_start) = title_copy.find(HYPERLINK_MARKER) {
+                let hyperlink_part = &title_copy[marker_start + HYPERLINK_MARKER.len()..];
+
+                // Find the closing double quote of the hyperlink
+                if let Some(quote_end) = hyperlink_part.find('"') {
+                    let before_hyperlink = &title_copy[..marker_start];
+                    let after_hyperlink = &hyperlink_part[quote_end + 1..];
+                    title_copy = format!("{}{}", before_hyperlink, after_hyperlink);
+                } else {
+                    // Sometimes links are broken, in these cases we only consider what is before the mark
+                    title_copy = title[..marker_start].to_string();
+                }
+            } else {
+                break;
+            }
+        }
+
+        title_copy
+    }
 }
 
 /// A page title.

packages/onenote-converter/src/section.rs

@@ -64,15 +64,14 @@ impl Renderer {
                 let _ = unsafe { write_file(&page_path, page_html.as_bytes()) };
 
                 let page_path_without_basedir =
-                    unsafe { remove_prefix(page_path.as_str(), output_dir.as_str()) }
+                    unsafe { remove_prefix(page_path, output_dir.as_str()) }
                         .unwrap()
                         .as_string()
                         .unwrap();
                 toc.push((title, page_path_without_basedir, page.level()))
             }
         }
 
-        log!("Section finished rendering: {:?}", section.display_name());
         let toc_html = templates::section::render(section.display_name(), toc)?;
         let toc_file = unsafe {
             join_path(

packages/onenote-converter/src/utils.rs

@@ -82,7 +82,7 @@ extern "C" {
 
     #[wasm_bindgen(js_name = removePrefix, catch)]
     pub unsafe fn remove_prefix(
-        base_path: &str,
+        base_path: String,
         prefix: &str,
     ) -> std::result::Result<JsValue, JsValue>;