serenity
serenity copied to clipboard
SerenityOS
Ladybird text input display issue
Hello, I'm a University of Utah student working on Ladybird as part of my final project for CS 4560 Web Browser Internals. While using the browser, I found a bug in how it displays a search bar of certain web pages, specifically the length of the search bar (pictured below are how Chrome displays the search bar and how Ladybird displays the same search bar)
Chrome:
Ladybird:
I minimized the bug down to this simple html and css. Screenshots of how Chrome and Ladybird render this webpage are below
<style>
.sb-input{
width:100%;
}
.sb-nav__secondary{
position:absolute;
}
</style>
<div class="sb-nav__secondary">
<input class="sb-input"/>
</div>
Chrome:
Ladybird:
I am interested in taking a stab at resolving this issue. I am not super familiar with the code yet, but Iād appreciate any pointers on how to proceed if anyone has ideas, thanks!
both <input> and element wrapping it create block formatting context (BFC). you could find that information by checking a dump of layout tree:
6847189.053 WebContent(39357): Viewport <#document> at (0,0) content-size 1321x836 [0+0+0 1321 0+0+0] [0+0+0 836 0+0+0] children: not-inline
BlockContainer <html> at (0,0) content-size 1321x836 [0+0+0 1321 0+0+0] [0+0+0 836 0+0+0] [BFC] children: not-inline
BlockContainer <body> at (8,8) content-size 1305x0 [8+0+0 1305 0+0+8] [8+0+0 0 0+0+8] children: inline
BlockContainer <div.sb-nav__secondary> at (8,8) content-size 6x22 positioned [0+0+0 6 0+0+0] [0+0+0 22 0+0+0] [BFC] children: inline
frag 0 from BlockContainer start: 0, length: 0, rect: [9,9 4x20] baseline: 22
TextNode <#text>
BlockContainer <input.sb-input> at (9,9) content-size 4x20 inline-block [0+1+0 4 0+1+0] [0+1+0 20 0+1+0] [BFC] children: not-inline
Box <div> at (11,10) content-size 0x18 flex-container(row) [0+0+2 0 2+0+0] [0+0+1 18 1+0+0] [FFC] children: not-inline
BlockContainer <div> at (11,10) content-size 0x18 flex-item [0+0+0 0 0+0+0] [0+0+0 18 0+0+0] [BFC] children: inline
TextNode <#text>
TextNode <#text>
TextNode <#text>
input has display: inline-block because of default.css so it sized in InlineFormattingContext::dimension_box_on_line()
a good next step for fixing the bug might be to find out why BFC and IFC end up assigning wrong width.
Random guess... Does putting the parent in absolute position trigger shrink to fit layout? Then the width: 100% doesn't resolve correctly because the parent width is not set yet? So it ends up set to 0?
I think this happens because of the following mechanism in HTMLInputElement::adjust_computed_style():
if (type_state() != TypeAttributeState::FileUpload) {
if (style.property(CSS::PropertyID::Width)->has_auto())
style.set_property(CSS::PropertyID::Width, CSS::LengthStyleValue::create(CSS::Length(size(), CSS::Length::Type::Ch)));
}
For input elements with a non-auto CSS width computed value, we take the HTMLInputElement::size() value and turn it into a CSS length with unit ch.
In your repro above, there's no size attribute on the input element, so we default to 20 per the spec.
Because there is also a width: 100% (non-auto) that applies to the input element, we just bypass the mechanism that would give us 20ch.
Note that HTMLInputElement::adjust_computed_style is a totally ad-hoc mechanism and not something from the spec. We needed a way to turn the size attribute into width: Nch and this is it. Sounds like we need to think of a better way!
One possible approach to this would be to let input elements have a natural/intrinsic layout size, derived from the size attribute and defaulting to 20ch. That would require special-casing input elements in layout width calculation.
Thank you all for the ideas! My team and I have started working on this issue and we found that moving the line
style.set_property(CSS::PropertyID::Width, CSS::LengthStyleValue::create(CSS::Length(size(), CSS::Length::Type::Ch)));
up out of the if statement fixes the specific web page in the original post. However, this obviously causes a lot of text inputs on other web pages to be formatted incorrectly. We are trying to add an if statement to the beginning of the adjust_computed_style method, something like
if (parent HTML element has position CSS property and position value is 'absolute')
style.set_property(CSS::PropertyID::Width, CSS::LengthStyleValue::create(CSS::Length(size(), CSS::Length::Type::Ch)));
however, we haven't found a way to access the parent HTML element nor its stylings in the adjust_computed_style method. If that approach seems reasonable, any advice on that would be appreciated.
We also looked into the idea mentioned in the post right before this ("special-casing input elements in layout width calculation."), but we haven't found where exactly the layout width calculator presently takes place, so if that approach is preferred we'd appreciate any direction on where to look for that.
Thank you again for your help!
This is definitely a non-trivial issue, perhaps not suitable for newcomers to the codebase.
Width calculation is not performed in a single location in layout, there are many different places and ways we determine widths, depending on a number of circumstances (which formatting context is used, the type of box being sized, etc)
One way that might work is to piggyback on replaced element layout by making box_is_sized_as_replaced_element() return true for boxes representing input elements. We would then also need those boxes to be assigned natural width and height somehow (via set_natural_width and set_natural_height so that they can get the <size>ch width and 1lh height you'd expect from <input type=text>
This specific behavior makes sense for text-based input elements, but we'd need different behavior for <input type=button>, <input type=image> etc.
Thanks @awesomekling, we'll try that. It seems to make sense, input elements usually are rendered as replaced content with natural width and height on other OSes. Buttons and images are too but agreed that it's less clear exactly how big those should be.
