Skip to content

/ servo

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.

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 border does not enclose inline-block #7681

Open
notriddle opened this issue Sep 18, 2015 · 5 comments
Open

Inline border does not enclose inline-block #7681

notriddle opened this issue Sep 18, 2015 · 5 comments
Labels
A-layout/inline C-has-manual-testcase

Comments

@notriddle
Copy link
Contributor

@notriddle notriddle commented Sep 18, 2015 

<!DOCTYPE html>
<html>
<head>
<style>
    .outer {border:solid 1px}
    .inner {display:inline-block}
</style>
</head>
<body>
    <span class="outer">
        <span class="inner">text</span>
        next
    </span>
</body>
</html>

screenshot from 2015-09-18 15-53-31
@jdm jdm added the A-layout/inline label Sep 19, 2015
@mbrubeck
Copy link
Contributor

@mbrubeck mbrubeck commented Apr 4, 2016

I think this code is supposed to draw the border when building the display list for span#outer, but is incorrectly using node.style from the child node, rather than self.style.
@notriddle
Copy link
Contributor Author

@notriddle notriddle commented Apr 4, 2016

Are you sure about that? As a reminder, <span style="border:red"><span style="border:black">TEST</span></span> is expected to work, too, even though there will only be one fragment (inline elements don't generate flows, and fragments are never nested). I'm pretty sure that's what the inline_contexts.nodes list is for.
@mbrubeck
Copy link
Contributor

@mbrubeck mbrubeck commented Apr 4, 2016

Oh yes, I misunderstood what that code was doing.
@notriddle
Copy link
Contributor Author

@notriddle notriddle commented Apr 4, 2016

I pretty much understand what the problem is: the inner block flow isn't drawing a border, because there's nothing that would make it do so, and the fragment isn't drawing a border because it's not a primary fragment.

If I remove the is_primary_fragment conditional, it draws the rest of the border... sort of. It's offset wrong, probably for the same reason (if you look closely) that the text is already off the baseline.
@notriddle notriddle mentioned this issue Apr 5, 2016
Merged
bors-servo added a commit that referenced this issue Apr 6, 2016
@bors-servo
Auto merge of #10419 - notriddle:whitespace_border, r=mbrubeck
Loading status checks…
cb943b0 
Whitespace stripping should not result in a dangling, open border.

No open issue (found it while working on #7681).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10419)
<!-- Reviewable:end -->
@mbrubeck mbrubeck mentioned this issue Apr 12, 2016
Closed
@notriddle notriddle mentioned this issue Sep 4, 2016
Closed
@nox
Copy link
Member

@nox nox commented Sep 30, 2017

This is still true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-layout/inline C-has-manual-testcase
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@mbrubeck @jdm @nox @notriddle
You can’t perform that action at this time.