Improve handling HTML special cases #312
Conversation
…s source tokens I do need those continuation delimiters for that, even though I really don't like them since they're so character set focussed!
Tag case is retained in the output though. Well, for the opening tag at least. Closing tag always matches opening tag.
Fixes #339
std::bad_alloc :( Also expand tests to make sure we're recording the full ignored tag contents.
Trouble was that `Scanner::scanEntity()` returns a value() that does not point to inside the HTML input stream (but to a *decoded* entity instead). So we need another API, `Scanner::start()`, to figure out where a token starts in HTML.
When a word near the of a translated sentence aligns with one at the beginning, it pushes prevIt back to the beginning. Then the next translated token will insert all straggler void elements between prevIt and it. Instead of using prevIt to track where we were with inserting stragglers, we keep our own iterator that never moves backwards.
…e after a tag Main reason for using this instead of `std::isspace` is to prevent a space being inserted between the tag and the full stop in `This is a <b>test</b>.`. Because that has been bothering me a lot.
# Conflicts: # src/translator/html.cpp
These are all elements that Firefox treats as opaque in their HTML5 parser. As a consequence, when you'd request `noscriptElement.innerHTML` you'd get the raw text content of the thing, as opposed to a serialized tree. So invalid HTML? Just passed on as is! Well, we're going to do the same then. Besides, if noscript then also probably no extension.
This tag is a bit difficult. No HTML is allowed inside of it (e.g. similar to `<textarea>`) but we do want to capture it's text content as text (decoding entities etc.) so we can translate it. So for now I'll just trust that nobody is insane enough to use HTML inside the title tag. And if they do, we'll be as insane back and try to maintain that (very much not allowed) structure.
I don't know what happened here.
Hopefully this will make the overall code more readable given you're familiar with the concept it tries to implement…
|
I think this pull request has grown enough to start trying to clean it up and get it merged. Preferably, #353 can be landed, before this gets merged. I will add some measures that show that this set of changes improves the HTML output quality. The description above mostly cover it: edge cases and refactoring to keep growing code complexity manageable. I've tried to make HTML.cpp as readable as possible by leaving comments with my intentions throughout, but clang-tidy still gives be very, very low grades for certain functions (especially those with switch statements.) Please let me know if any bit of code's intended use isn't clear, or if you have suggestions on how I can split up certain parts to be more comprehensible. |
Let's take this PR forward before #353. #353 looks like it will take longer - it's still exploratory. Let's discuss internally how to craft and position the numbers. |
|
I'll try to add a more elaborate test to bergamot-translator-tests to test some of the added edge cases as well. |
There was a problem hiding this comment.
Leaving a few comments that are local to the source around in a first pass. I expect to take one more pass once my understanding of the HTML pipeline improves from exchanges here.
I will try to do some black-box testing via the extension (as opposed to the previous python, thanks @jelmervdl for the dogfooding capabilities and improvements consequent of experimenting with the extension to the library).
Overall positive about this PR, so hoping to expedite merge. Most of the queries and suggestions below may be undertaken follow-up PR (save a suspected infinite loop).
There was a problem hiding this comment.
Thanks for the extra tests. Let's get this in, and pursue things below in later PRs.
As feedback, that needn't necessarily be covered in this PR - someone trying to understand the HTML part will appreciate a bigger blob of documentation near the HTML class or something describing the components (parsing, what form of AST/DOM equivalent, target HTML gen/restoration) - at least I would.
Minor: I do not fully understand what stragger means (tried googling) in this context. I see most of the Taint (which I had troubled comprehending before) has been replaced with TagStack.
Long term I think it will be a good idea to abstract different readers writers inserting something pandoc like in between, thus providing a framework to translate formats (docx, markdown, LaTeX etc). Right now we're specializing for HTML.
jerinphilip
changed the title
Draft for now to just capture ongoing improvements
Change log
<p><b>Cayo</b> o <b>Gayo Trebacio Testa</b> (m. ca. 4) nacido probablemente en <a href="/wiki/Velia" title="Velia">Velia</a>, <a href="/wiki/Basilicata" title="Basilicata">Lucania</a>, fue uno de los abogados más prominentes de su época. Amigo de <a href="/wiki/Cicer%C3%B3n" title="Cicerón">Cicerón</a>, fue recomendado por este a <a href="/wiki/Julio_C%C3%A9sar" title="Julio César">César</a>, quien lo acogió en la <a href="/wiki/Galia" title="Galia">Galia</a> como consejero legal y jefe de la oficina de comunicaciones. Apoyó a César en la <a href="/wiki/Segunda_guerra_civil_de_la_Rep%C3%BAblica_romana" title="Segunda guerra civil de la República romana">guerra civil</a> y tras su asesinato pasó al bando de Octavio (futuro <a href="/wiki/Augusto" title="Augusto">Augusto</a>), quien tuvo en gran estima a Trebacio hasta que este falleció en el 4 d. C. Escribió varias obras relativas al <a href="/wiki/Derecho_romano" title="Derecho romano">Derecho romano</a>, pero ninguna se ha conservado.</p>(with es->en model)<code>and<samp>Pass through for certain HTML elements #313<wbr>as word-breaking <wbr> elements should not introduce spaces around them #339<P></p>(I haven't seen this go wrong but in theory it could have gone wrong.)isContinuationlogic to detect when to insert a space after an open or close tag. This will add a space inun<u>der</u>line, but not inend of the <b>sentence</b>.This change makes especially Wikipedia look more natural.<noscript>at the parser level because Firefox does so as well. Since Firefox can't guarantee that<noscript>tags have valid HTML inside them, this looks like the safest bet for now.