Closed Bug 1229212 Opened 4 years ago Closed 4 years ago

<input type="number"> has a non-block-level flex item (the text control frame)

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file, 1 obsolete file)

STR:
 1. Apply attachment 8692015 [details] [diff] [review] on top of current inbound tip:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/de171af8720b

 2. Compile a debug build.

 3. Load this URL in that debug build:
       data:text/html,<input type="number">

ACTUAL RESULTS: You'll trip one of the assertions in that patch.

EXPECTED RESULTS: No such assertion failures -- we should be able to assume that flex items are block-level.


BACKGROUND:
As I noted in bug 1176793 comment 20, we suppress flex-item-blockification inside of form controls, so that we don't accidentally blockify styles for the mandatory XUL junk that lives inside of e.g. <audio> just because someone happens to style it as "display:flex".  This suppression was added in bug 812822 and bug 844529, I believe.

So, <input type="number"> doesn't benefit from this auto-blockification. So, it should make sure its frames are block-level by hardcoding them as such, for any children that risk being anything other than block-level.

In this case, it's just its nsTextControlFrame (:-moz-number-text) which is incorrectly inline-level.  We can fix the assertion by giving that frame "display:block".
Attached patch fix v1 (obsolete) — Splinter Review
Attached patch fix v2Splinter Review
(Just tweaked patch with s/should/must/, RE flex items being blocks.)

Mats, I'm hoping this means your assertions in bug 1176793 can stay in.  Tagging you to review.
Attachment #8693846 - Attachment is obsolete: true
Attachment #8693853 - Flags: review?(mats)
Comment on attachment 8693853 [details] [diff] [review]
fix v2

OK, I'll leave the assertions in if the tests pass with this fix.
Attachment #8693853 - Flags: review?(mats) → review+
Looks like the tests do indeed pass with this fix -- hooray!  Successful try run (linux debug only):
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=d47a2f6401a4
Flags: in-testsuite-
Blocks: 1176793
https://hg.mozilla.org/mozilla-central/rev/f647c855f10b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Blocks: 1199229
Duplicate of this bug: 1199229
You need to log in before you can comment on or make changes to this bug.