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

<input type=text> does not size itself properly with respect to character width #4378

Open
mttr opened this issue Dec 15, 2014 · 15 comments
Open

<input type=text> does not size itself properly with respect to character width #4378

mttr opened this issue Dec 15, 2014 · 15 comments
Labels
A-layout/uncategorized I-wrong An incorrect behaviour is observed.

Comments

@mttr
Copy link
Contributor

mttr commented Dec 15, 2014

Test case:

<body>
<div><input type=text size=10 value="01234567890"></div>
<div><input type=text size=20 value="012345678901234567890"></div>
</body>

The value of both inputs should fit, but in the first example "0" overflows, and in the second one, "90" overflows.
@jdm jdm added A-layout/uncategorized I-wrong An incorrect behaviour is observed. labels Dec 15, 2014
@mttr mttr mentioned this issue Dec 15, 2014
Closed
bors-servo pushed a commit that referenced this issue Dec 15, 2014
auto merge of #4380 : mttr/servo/textarea_rows_and_cols, r=pcwalton
ef90fdc 
...with a bit of a caveat: sizing has the same problem as seen in #4378, and it is _significantly_ more noticeable when using `rows`.

Fixes #4291
@mttr
Copy link
Contributor Author

mttr commented Dec 16, 2014

This issue has gotten worse since originally posting: first input "90" overflows, second input "890" overflows.

bors-servo pushed a commit that referenced this issue Dec 16, 2014
auto merge of #4380 : mttr/servo/textarea_rows_and_cols, r=jdm
fcaa45f 
...with a bit of a caveat: sizing has the same problem as seen in #4378, and it is _significantly_ more noticeable when using `rows`.

Fixes #4291
@hirschenberger
Copy link
Contributor

The width of the input is calculated here:
https://github.com/servo/servo/blob/master/components/style/values.rs#L739
but the calculation is not as specified in HTML5 §14.5.4 by usung the average character width but by using constant 0.5.
I'd like to fix it if someone can give me a hint, how to get the corresponding gfx::font::FontMetrics?

@jdm
Copy link
Member

jdm commented Feb 12, 2015

My initial guess is that we want another argument for compute_Au_with_font_size that would have the correct value, but I got stuck trying to find somewhere sensible to get that value and store it in the Context or ComputedValue structs. @SimonSapin or @pcwalton, do you have thoughts about the previous comment?

@hirschenberger
Copy link
Contributor

There's a value: average_advance in gfx::font::FontMetrics which I think is the value in question. That value is in FontContext which is in LayoutContext, but I haven't found a way to get the current LayoutContext within the style-stuff where compute_Au_with_font_size lives.

@jdm
Copy link
Member

jdm commented Feb 12, 2015

Yes, that's the problem I was trying to resolve, too. The code in style/ doesn't have any knowledge of actual fonts or metrics, and it would be best to find ways to inject specific values from outside into it.

@jdm
Copy link
Member

jdm commented Feb 12, 2015

For what it's worth, the code that initiates style recalculation for nodes is http://mxr.mozilla.org/servo/source/components/layout/traversal.rs#130 and the RecalcStyleForNode structure contains a layout context member.

@jdm
Copy link
Member

jdm commented Feb 12, 2015

The relevant code there is the node.cascade_node, which propagates style information down through the children and computes sizes.

@hirschenberger
Copy link
Contributor

Ok, what do you think, passing the LayoutContext down the cascade functions?

@jdm
Copy link
Member

jdm commented Feb 12, 2015

That seems like an unlikely solution, not least because the layout crate depends on the style crate, and LayoutContext is defined in the layout crate. Maybe we want a special CascadeContext for things like this.

@jdm
Copy link
Member

jdm commented Feb 12, 2015

That being said, it would be important to figure out if the FontContext in LayoutContext is even meaningful at the point that the cascade occurs.

@jdm
Copy link
Member

jdm commented Feb 13, 2015

@SimonSapin might have ideas here, since a similar thing came up on IRC: http://logs.glob.uno/?c=mozilla%23servo#c172155

@SimonSapin
Copy link
Member

It makes sense for libstyle to know about font metrics (though maybe not all of LayoutContext) at "computed value" time. That’s also needed for the ch unit.

@hirschenberger
Copy link
Contributor

This descision/refactoring seems to go beyond my capabilities as a newbie.
Ich tried to pass the FontContext to the cascade functions, but got a circular crate dependency between style and gfx.

@jdm
Copy link
Member

jdm commented Feb 17, 2015

Yeah, we probably need to define a computed-style-specific structure in libstyle that layout could use.

@frewsxcv frewsxcv mentioned this issue Dec 2, 2015
Closed
@dralley
Copy link
Contributor

dralley commented Apr 20, 2020 •
edited

Still a problem, although it seems worse now (more characters obscured than supposedly were when first reported)

Screenshot from 2020-04-20 19-51-06

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/uncategorized I-wrong An incorrect behaviour is observed.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jdm @SimonSapin @hirschenberger @mttr @dralley