Skip to content

Fix blockquote and dd having textblocks double added #405

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
Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix blockquote and dd having textblocks double added #405

wants to merge 2 commits into from

Conversation

@campaul
Copy link
Contributor

@campaul campaul commented May 29, 2025

Test for #404

@campaul campaul force-pushed the floating_blockquote_test branch from e5048d4 to 62be8a6 Compare May 29, 2025 22:37
@campaul
Copy link
Contributor Author

campaul commented Jun 2, 2025

The Html_tag_open_blockquote and Html_tag_open_dd were calling Html_add_textblock, essentially turning the elements into block elements despite them being inline. Following that, if a style was applied that would actually make the element be block (display: block, display: inline-block, or float: left/right), then it would end up having the textblock double added.

To fix this I removed both of those functions and replaced them with calls to Html_tag_open_default. I then added blockquote and dd to the list of elements that have display: block applied by default which is recommended by the CSS specification and what other browsers do.

@campaul campaul changed the title Add test for floating blockquote Fix blockquote and dd having textblocks double added Jun 2, 2025
@rodarima
Copy link
Member

Thanks, I was trying to find out why it was being done that way. I traced it back to the first commit we have recoded from 2007 which already used the textblock (my comments apply to dd too):

dillo/src/html.cc

Lines 2770 to 2774 in 93715c4

static void Html_tag_open_blockquote(DilloHtml *html, char *tag, int tagsize)
{
DW2TB(html->dw)->addParbreak (9, S_TOP(html)->style);
Html_add_indented(html, 40, 40, 9);
}

dillo/src/html.cc

Lines 639 to 643 in 93715c4

static void Html_add_indented(DilloHtml *html, int left, int right, int space)
{
Textblock *textblock = new Textblock (false);
Html_add_indented_widget (html, textblock, left, right, space);
}

At that point there was no logic to handle display elements like we have now:

dillo/src/html.cc

Lines 4094 to 4119 in f6abc11

if (! S_TOP(html)->display_none) {
switch (html->style ()->display) {
case DISPLAY_BLOCK:
Html_display_block(html);
break;
case DISPLAY_INLINE_BLOCK:
Html_display_inline_block(html);
break;
case DISPLAY_LIST_ITEM:
Html_display_listitem(html);
break;
case DISPLAY_NONE:
S_TOP(html)->display_none = true;
break;
case DISPLAY_INLINE:
if (html->style()->vloat != FLOAT_NONE)
Html_display_block(html);
break;
default:
break;
}
if (Tags[ni].content && ! S_TOP(html)->display_none) {
Tags[ni].content (html, tag, tagsize);
}
}

I'm guessing this would only potentially cause problems if we have an inline blockquote? As before we would have a new textblock but now we won't.

Here is a testcase with inline blockquote:

<!DOCTYPE html>
<html>
  <head>
    <title>Test inline blockquote</title>
    <style type="text/css">
      div {
        background-color: #ededed;
        margin: 10px 3em;
        padding: 15px;
        border-radius: 5px;
      }
      blockquote {
        display: inline;
        border: solid 2px black;
      }
    </style>
  </head>
  <body>
    <div>
      <blockquote cite="https://www.huxley.net/bnw/four.html">
        Words can be like X-rays, if you use them properly—they’ll go through
        anything. You read and you’re pierced.
      </blockquote>
      <p style="text-align:right">—Aldous Huxley, <cite>Brave New World</cite></p>
    </div>
  </body>
</html>

And here it how it changes the rendering from this branch, master and Firefox (top to bottom):

bq

Not sure if there is a good way to avoid breaking the current behavior for inline. We don't follow the "correct" inline behavior either.

@campaul
Copy link
Contributor Author

campaul commented Aug 8, 2025 •
edited
Loading

Not sure if there is a good way to avoid breaking the current behavior for inline. We don't follow the "correct" inline behavior either.

It looks like it wont be too hard to implement the correct border behavior for inline elements, so I'm going to focus on implementing that first then return to this PR after that's done.

@rodarima
Copy link
Member

I'm going to focus on implementing that first then return to this PR after that's done.

I'll mark this as WIP in the meanwhile.

@rodarima rodarima marked this pull request as draft August 11, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@campaul @rodarima