Page MenuHomePhabricator

Remarkup: Correctly render inline embed layout
ClosedPublic

Authored by matmarex on Feb 6 2015, 10:56 PM.
Referenced Files
Unknown Object (File)
Tue, Apr 16, 3:01 PM
Unknown Object (File)
Sat, Mar 30, 3:15 AM
Unknown Object (File)
Mar 5 2024, 11:07 PM
Unknown Object (File)
Mar 5 2024, 11:07 PM
Unknown Object (File)
Feb 27 2024, 8:37 PM
Unknown Object (File)
Feb 27 2024, 7:42 PM
Unknown Object (File)
Feb 24 2024, 9:07 PM
Unknown Object (File)
Feb 14 2024, 12:22 PM
Subscribers

Details

Summary

The generated HTML is like <p>some text <div …>…</div> more text</p>, and HTML <p/> tags may not contain block content like <div/> tags. Browsers actually parse this as if it was <p>some text </p><div …>…</div> more text<p></p> (sic).

The layout CSS class already has display: inline set, but this is not sufficient. Browser's HTML parser doesn't care what CSS rules will be applied, it only deals with the meanings of tags.

Fixes T7201.

Test Plan

Verify that the following displays the image inline:

some text {Fnnn,layout=inline} more text

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

matmarex retitled this revision from to Remarkup: Correctly render inline embed layout.
matmarex updated this object.
matmarex edited the test plan for this revision. (Show Details)
matmarex added reviewers: epriestley, chad.
matmarex set the repository for this revision to rP Phabricator.
matmarex added a project: Remarkup.
epriestley edited edge metadata.
  • Let's make the $layout_class imply display: inline instead of actually changing the markup.
  • After making a CSS change, run bin/celerity map to rebuild the static resource map.
This revision now requires changes to proceed.Feb 9 2015, 2:15 PM

It already implies display: inline in CSS, but as I have explained on the task that is not sufficient. Browser's HTML parser doesn't care what CSS rules will be applied, it only deals with the meanings of tags, and <p/> is specified as not being allowed to have a <div/> inside it. This is the same kind of thing as if you tried to nest <a/> tags, it's just not possible.

matmarex edited the test plan for this revision. (Show Details)
matmarex edited edge metadata.
epriestley edited edge metadata.

Ah, sorry I missed that. Thanks!

This revision is now accepted and ready to land.Feb 9 2015, 3:52 PM
This revision was automatically updated to reflect the committed changes.