Skip to content

Commit

Permalink
Desktop: Fixes #11617: Links from imported notes from OneNote were be…
Browse files Browse the repository at this point in the history 
…ing wrongly rendered (#11618)
  • Loading branch information
@pedr
pedr authored Jan 9, 2025
1 parent ab286b6 commit 86e6445
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 7 deletions.
Binary file added packages/app-cli/tests/support/onenote/hyperlink_marker_as_first_character.zip
View file
Binary file not shown.
11 changes: 11 additions & 0 deletions packages/lib/services/interop/InteropService_Importer_OneNote.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,4 +196,15 @@ describe('InteropService_Importer_OneNote', () => {
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);
});
});
139 changes: 139 additions & 0 deletions packages/lib/services/interop/__snapshots__/InteropService_Importer_OneNote.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,3 +1028,142 @@ exports[`InteropService_Importer_OneNote should remove hyperlink from title: 风
</body>
</html>"
`;
exports[`InteropService_Importer_OneNote should render links properly by ignoring wrongly set indices when the first character is a hyperlink marker: Is Mexico safe for shooting Street Photography 1`] = `
"<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Is Mexico safe for shooting Street Photography?</title>
<style>
* { margin: 0; padding: 0; font-weight: normal; }
table, tr, td { border-color: #A3A3A3; }
ul, ol { padding: 0; }
.title .outline-element { display: inline; }
.title .outline-element:nth-child(2) { margin-left: 10px !important; }
.container-outline { font-family: Calibri, sans-serif; font-size: 6pt; }
.ink-text, .ink-space { display: inline-block; position: relative; vertical-align: bottom; }
.ink-text { top: 0; left: 0; }
.note-tag-icon { position: relative; }
.note-tag-icon > svg { position: absolute; }
.icon-secondary > svg { position: absolute; fill: black; filter: drop-shadow(0 0 2px white); height: 12px; top: -1px; }
.icon-secondary > .content { position: absolute; color: black; filter: drop-shadow(0 0 2px white); font-size: 10px; color: black; top: -1px; user-select: none; }
</style>
</head>
<body>
<div class="title" style="left: 48px; position: absolute; top: 24px;"><div class="container-outline" style="width: 624px;"><div class="outline-element" style="margin-left: 0px;"><span style="font-family: Calibri; font-size: 20pt; line-height: 32px;">&nbsp;</span></div>
</div><div class="container-outline" style="width: 624px;"><div class="outline-element" style="margin-left: 0px;"><span style="color: rgb(118,118,118); font-family: Calibri; font-size: 10pt; line-height: 16px;">Monday, August 28, 2023</span></div>
<div class="outline-element" style="margin-left: 0px;"><span style="color: rgb(118,118,118); font-family: Calibri; font-size: 10pt; line-height: 16px;">10:52 AM</span></div>
</div></div><div class="container-outline" style="left: 48px; position: absolute; top: 115px; width: 624px;"><div class="outline-element" style="margin-left: 0px;"><p style="font-family: Calibri; font-size: 12pt; line-height: 19px;"><a href="https://www.youtube.com/watch?v=-U-uj1jaHtk" style="">Is Mexico safe for shooting Street Photography?</a></p></div>
<div class="outline-element" style="margin-left: 0px;"><p style="font-family: Calibri; font-size: 12pt; line-height: 19px;">&nbsp;</p></div>
<div class="outline-element" style="margin-left: 0px;"><img src=":/5" style="max-height: 200px; max-width: 356px;" /></div>
</div>
<script>
if (window.parent !== null) {
window.parent.postMessage(window.location.href, '*');
}
</script>
</body>
</html>"
`;
exports[`InteropService_Importer_OneNote should render links properly by ignoring wrongly set indices when the first character is a hyperlink marker: Quick Notes 1`] = `
"<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Quick Notes</title>
<style>
html, body { margin: 0; padding: 0; }
body {
display: flex;
}
nav {
height: 100vh;
min-width: 200px;
max-width: 300px;
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol";
border-right: 1px solid rgb(235, 235, 235);
}
nav ul {
padding: 0;
margin: 0;
overflow-y: auto;
height: 100%;
}
nav li {
padding: 10px 20px;
border-bottom: 1px solid rgb(235, 235, 235);
text-overflow: ellipsis;
overflow: hidden;
white-space: nowrap;
}
nav li.active {
background-color: rgb(233, 233, 233);
}
nav a {
color: black;
text-decoration: none;
}
.content {
flex: 1;
}
</style>
</head>
<body>
<nav>
<ul>
<li class="l1"><a href=":/3" target="content" title="Is Mexico safe for shooting Street Photography?">Is Mexico safe for shooting Street Photography?</a></li>
</ul>
</nav>
<iframe src="" frameborder="0" name="content" class="content"></iframe>
<style>
.l2 { padding: 10px 20px 10px 40px }
.l3 { padding: 10px 20px 10px 60px }
.l4 { padding: 10px 20px 10px 80px }
.l5 { padding: 10px 20px 10px 100px }
</style>
<script>
document.addEventListener('click', function (event) {
// If the clicked element doesn't have the right selector, bail
if (!event.target.matches('nav a')) return;
for (const child of event.target.parentElement.parentElement.children) {
child.classList.remove('active');
}
event.target.parentElement.classList.add('active');
}, false);
window.addEventListener('message', (event) => {
const activeTarget = event.data;
for (const link of document.querySelectorAll('nav ul li a')) {
if (link.href === activeTarget) {
link.parentElement.classList.add('active');
}
}
});
if (window.parent !== null) {
window.parent.postMessage(window.location.href, '*');
}
</script>
</body>
</html>"
`;
32 changes: 25 additions & 7 deletions packages/onenote-converter/src/page/rich_text.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ impl<'a> Renderer<'a> {
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
// https://github.com/laurent22/joplin/issues/11617
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));
}
Expand Down Expand Up @@ -108,11 +122,12 @@ impl<'a> Renderer<'a> {
.zip(styles.iter())
.map(|(text, style)| {
if style.hyperlink() {
let result = self.render_hyperlink(text.clone(), style, in_hyperlink, is_href_finished);
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)
Ok(result.unwrap().0)
} else {
Ok(text)
}
Expand Down Expand Up @@ -157,7 +172,10 @@ impl<'a> Renderer<'a> {
let url_2 = url.strip_suffix('"');

if url_2.is_some() {
return Ok((format!("<a href=\"{}\" style=\"{}\">", url_2.unwrap(), style), true));
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))
Expand All @@ -172,10 +190,10 @@ impl<'a> Renderer<'a> {
Ok((text, false))
}
} else {
Ok((format!(
"<a href=\"{}\" style=\"{}\">{}</a>",
text, style, text
), true))
Ok((
format!("<a href=\"{}\" style=\"{}\">{}</a>", text, style, text),
true,
))
}
}

Expand Down

0 comments on commit 86e6445

Please sign in to comment.