Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inline Translation adds <span> within <head> #939

Closed
Vinai opened this issue Jan 9, 2015 · 2 comments
Closed

Inline Translation adds <span> within <head> #939

Vinai opened this issue Jan 9, 2015 · 2 comments
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@Vinai
Copy link
Contributor

Vinai commented Jan 9, 2015

This issue is exposed by the integration test Magento\DesignEditor\Model\Translate\InlineTest::testProcessResponseBodyHtml in the current develop branch.
I don't know why it runs successfully in travis, but the failure seems correct to me, so that is the reason for this issue.
I'm unsure what the correct behavior should be, so I'll refrain from creating a PR with a fix. If you help me understand how the following issue should be handled, I might work on one in my quest get all integration tests passing locally.

The test uses the following fixture:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
        "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
    <title>{{{shown_0}}{{translated_0}}{{original_0}}{{scope_0}}}</title>
</head>
<body>
    <div title="some_title_{{{shown_1}}{{translated_1}}{{original_1}}{{scope_1}}}_in_div">
    some_text_{{{shown_2}}{{translated_2}}{{original_2}}{{scope_2}}}_in_div
    </div>
</body>
</html>

During the test, the title is transformed by the method Magento\DesignEditor\Model\Translate\Inline\Parser::_specialTags() from

<head>
    <title>{{{shown_0}}{{translated_0}}{{original_0}}{{scope_0}}}</title>
</head>

to

<head>
    <title>shown_0</title><span class="translate-inline-title" data-translate="[{&quot;shown&quot;:&quot;shown_0&quot;,&quot;translated&quot;:&quot;translated_0&quot;,&quot;original&quot;:&quot;original_0&quot;,&quot;location&quot;:&quot;Page title&quot;}]" data-translate-mode="text"></span>
</head>

The problem is that the <head> now contains a new <span> node.
This causes the following code in the test method to fail:

        $actual = new \DOMDocument();
        $actual->preserveWhiteSpace = false;
        $actual->loadHTML($actualText);

The exception message is
DOMDocument::loadHTML(): Unexpected end tag : head in Entity, line: 11

The reason why I think this happens is because according to http://www.w3.org/TR/html401/struct/global.html#h-7.4.1 the closing </head> tag is optional.
Because of this, it seems that as soon as the parser encounters the <span> element after <title>, it markes the <head> as implicitly closed, and the <body> as the current context, and thus chokes on the following closing </head>.

So at least for that doctype there must be no <span> within <head>.
No idea how inline translation of the page title should be handled instead.
@Vinai Vinai mentioned this issue Jan 9, 2015
Closed
@monkeysee
Copy link
Contributor

Internal tracking number: MAGETWO-33025

@muasir muasir added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Jan 23, 2015
@cspruiell
Copy link
Contributor

The DesignEditor module no longer exists, so this issue is obsolete.

magento-team pushed a commit that referenced this issue Mar 21, 2017
@rganin
Merge pull request #939 from magento-troll/MAGETWO-65182
a4b34fe 
MAGETWO-65182: Remove \Magento\Eav\Model\Entity\Attribute\Backend\Serialized class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Vinai @muasir @maksek @monkeysee @cspruiell