Closed
Bug 970257
Opened 11 years ago
Closed 11 years ago
Input type "number" has width of 0px when set to auto
Categories
(Core :: Layout: Form Controls, defect, P3)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: pot, Assigned: jwatt)
References
()
Details
(Keywords: testcase)
Attachments
(2 files)
|
(deleted),
image/png
|
Details | |
|
(deleted),
patch
|
dholbert
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Inputs render in different widths
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140205162153
Steps to reproduce:
I added an input[type="number"] element on my page and gave it "width: auto" in the CSS, then took a look at it in Firefox v28 (beta).
Actual results:
The input element got its width set at 0 and the contents wasn't visible.
Expected results:
The input element gets its width set in a similar matter as the input[type="text"] element, which gets its width set at 144px.
Component: Untriaged → Layout: Form Controls
Product: Firefox → Core
|
||
Updated•11 years ago
|
Flags: needinfo?(jwatt)
|
||
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 8.1 → All
Hardware: x86_64 → All
Version: 28 Branch → Trunk
|
|
Assignee | |
Comment 1•11 years ago
|
||
Just need to implement Get[Min|Pref]Width properly.
|
|
Assignee | |
Comment 2•11 years ago
|
||
The test would fail without the patch because the input would be so narrow that the right border and the spin buttons would be visible in the test (not hidden under the black div).
|
|
||
Comment 3•11 years ago
|
||
Comment on attachment 8374594 [details] [diff] [review]
patch
>+nscoord
>+nsNumberControlFrame::GetMinWidth(nsRenderingContext* aRenderingContext)
>+{
>+ nscoord result;
>+ DISPLAY_MIN_WIDTH(this, result);
>+
>+ nsIFrame* kid = mFrames.FirstChild();
>+ result = nsLayoutUtils::IntrinsicForContainer(aRenderingContext,
>+ kid,
>+ nsLayoutUtils::MIN_WIDTH);
We need to handle the possibility of 'kid' being null here (for 'display:none'), since IntrinsicForContainer assumes non-null input. I think it's probably reasonable to just return 0 in that case.
(Same goes for GetPrefWidth.)
It'd be nice to test that, too (at least, that we don't crash/assert), but we can't do that without styling the pseudo-elements, so that'll be another thing to test on bug 926412. I'll add a comment over there.
>+++ b/layout/forms/nsNumberControlFrame.h
>@@ -45,16 +45,20 @@ public:
> virtual void DestroyFrom(nsIFrame* aDestructRoot) MOZ_OVERRIDE;
[...]
>+ virtual nscoord GetMinWidth(nsRenderingContext *aRenderingContext) MOZ_OVERRIDE;
>+
>+ virtual nscoord GetPrefWidth(nsRenderingContext *aRenderingContext) MOZ_OVERRIDE;
>+
> NS_IMETHOD Reflow(nsPresContext* aPresContext,
Nit: Make those asterisks left-aligned, to match surrounding code and to match your impls of these methods.
>+++ b/layout/reftests/forms/input/number/number-auto-width-1.html
>@@ -0,0 +1,8 @@
>+<!DOCTYPE html>
>+<html>
>+ <body>
>+ <input type="number" style="-moz-appearance:none; width:auto;">
>+ <!-- div to cover spin box area -->
>+ <div style="display:block; position:absolute; background-color:black; width:2000px; height:100px; top:0px; left:100px;">
I prefer the "-moz-appearance:textfield" technique for hiding the spinner, that you used in bug 951310 -- that lets us actually verify that the width is exactly the same, instead of just that it's large enough to stick under the black div (i.e. > 100px)
So I'd lean towards switching to that, and dropping the black div.
r=me with the above.
Attachment #8374594 -
Flags: review?(dholbert) → review+
|
|
||
Comment 4•11 years ago
|
||
> for 'display:none'
Can the kid actually be styled with display:none in this case?
|
|
||
Comment 5•11 years ago
|
||
Only in chrome-privileged stylesheets for now, though IIUC we may expose the pseudo-element to the web eventually.
Also, we go to the trouble of checking whether the child is null in ::Reflow():
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsNumberControlFrame.cpp#88
and it doesn't really make sense to have a null-check there but not here.
|
|
Assignee | |
Comment 6•11 years ago
|
||
|
|
Assignee | |
Comment 7•11 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #3)
> I prefer the "-moz-appearance:textfield" technique for hiding the spinner,
> that you used in bug 951310 -- that lets us actually verify that the width
> is exactly the same, instead of just that it's large enough to stick under
> the black div (i.e. > 100px)
That's not possible since I'm comparing to a text control which doesn't necessarily have the same pref width as a number control. Other than that I addressed your comments.
|
||
Updated•11 years ago
|
Target Milestone: mozilla19 → mozilla29
|
||
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: mozilla29 → mozilla30
|
|
Assignee | |
Comment 9•11 years ago
|
||
Comment on attachment 8374594 [details] [diff] [review]
patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug in new feature (bug 344616)
User impact if declined: number controls broken if styled using auto width
Testing completed (on m-c, etc.): merged to m-c
Risk to taking this patch (and alternatives if risky): low risk, early in aurora
String or IDL/UUID changes made by this patch: none
Attachment #8374594 -
Flags: approval-mozilla-aurora?
|
|
||
Updated•11 years ago
|
Attachment #8374594 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
|
|
||
Updated•11 years ago
|
status-firefox30:
--- → fixed
|
|
Assignee | |
Comment 10•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•