Text input suddenly grows in size after typing certain amount of characters

RESOLVED FIXED in Firefox 14

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: dbaron)

Tracking

(Depends on: 1 bug, {regression, testcase})

Trunk
mozilla15
ARM
Android
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14+ fixed, blocking-fennec1.0 +)

Details

(Whiteboard: readability, URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 617427 [details]
testcase

Steps to reproduce:
- Visit testcase
- Type a letter (for example 'q') a couple of times


Expected result:
- The text input stays at the same size

Actual result:
- The text input suddenly grows in size after typing 8 times the letter 'q'.

Reproduce this on the Samsung Galaxy Nexus and the Samsung Galaxy SII

See also video:
http://www.youtube.com/watch?v=IgrpchjAtWQ

This regressed between 2012-04-17 and 2012-04-18, regression from bug 706193, I think.
(Reporter)

Comment 1

5 years ago
I'm also seeing the link suddenly grow when tapping somewhere on this page:
http://people.mozilla.org/~mwargers/tests/tapping/681640_tapevents.html

Updated

5 years ago
Blocks: 627842
No longer blocks: 706193
Whiteboard: readability
blocking-fennec1.0: ? → +
tracking-firefox14: --- → ?
Keywords: regression
(Assignee)

Updated

5 years ago
Assignee: nobody → dbaron
Blocks: 706193

Updated

5 years ago
tracking-firefox14: ? → +
(Assignee)

Comment 2

5 years ago
I wrote a patch for this on the airplane a few days ago, but then forgot to test that the tests pass and post it.  Hopefully I'll get back to that soon.
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/d87bc5f40a15/line-threshold-form-controls
(Assignee)

Comment 3

5 years ago
Created attachment 620436 [details] [diff] [review]
patch
Attachment #620436 - Flags: review?(roc)
Comment on attachment 620436 [details] [diff] [review]
patch

Review of attachment 620436 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsFontInflationData.cpp
@@ +316,5 @@
> +      nsIFrame *optionChild = option->GetFirstPrincipalChild();
> +      if (optionChild && optionChild->GetType() == nsGkAtoms::textFrame) {
> +        optionResult = nsTextFrameUtils::
> +          ComputeApproximateLengthWithWhitespaceCompression(
> +            optionChild->GetContent(), optionChild->GetStyleText());

Don't we need to iterate through the child frames of <option> in case there are multiple text nodes separated by comments?

@@ +380,5 @@
> +        // need to exclude the display frame.
> +        nscoord fontSize = kid->GetStyleFont()->mFont.size;
> +        PRInt32 charCount = CharCountOfLargestOption(
> +          static_cast<nsComboboxControlFrame*>(kid)->GetDropDown());
> +        mTextAmount += charCount * fontSize;

Is this really the right thing to do for non-auto-width replaced elements?
(Assignee)

Comment 5

5 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> ::: layout/generic/nsFontInflationData.cpp
> @@ +316,5 @@
> > +      nsIFrame *optionChild = option->GetFirstPrincipalChild();
> > +      if (optionChild && optionChild->GetType() == nsGkAtoms::textFrame) {
> > +        optionResult = nsTextFrameUtils::
> > +          ComputeApproximateLengthWithWhitespaceCompression(
> > +            optionChild->GetContent(), optionChild->GetStyleText());
> 
> Don't we need to iterate through the child frames of <option> in case there
> are multiple text nodes separated by comments?

I suppose so.

> @@ +380,5 @@
> > +        // need to exclude the display frame.
> > +        nscoord fontSize = kid->GetStyleFont()->mFont.size;
> > +        PRInt32 charCount = CharCountOfLargestOption(
> > +          static_cast<nsComboboxControlFrame*>(kid)->GetDropDown());
> > +        mTextAmount += charCount * fontSize;
> 
> Is this really the right thing to do for non-auto-width replaced elements?

I may at some future point want to try to consider widths on inline blocks and replaced elements, but for now I'm just trying to *not* consider form control inputs.
(Assignee)

Comment 6

5 years ago
Created attachment 620759 [details] [diff] [review]
patch

revised to iterate the children of <option>
Attachment #620436 - Attachment is obsolete: true
Attachment #620436 - Flags: review?(roc)
Attachment #620759 - Flags: review?(roc)
Attachment #620759 - Flags: review?(roc) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8125b89352d
Target Milestone: --- → mozilla15
Depends on: 751808
Sorry, backed this out for test failures in 40-50% of the compilable pushes since this landed (which is too high for me to be happy merging inbound into mozilla-central):

https://hg.mozilla.org/integration/mozilla-inbound/rev/308f1163a388

See bug 751808 for logs :-)
Target Milestone: mozilla15 → ---
Backed out: https://hg.mozilla.org/mozilla-central/rev/308f1163a388

Updated

5 years ago
Depends on: 752168
Just to help explain the crashes in bug 752168 - this had ended up being merged to mozilla-central for 3-4 hours before the backout was also merged (we had a 250+ changeset backlog on mozilla-inbound due to CPG, so it slipped through with the efforts to lower that as soon as possible), so ended up being included in the Nightly respin, which is how crashes were occurring in the wild. Today's Nightly will include the backout.
(Assignee)

Comment 11

5 years ago
(In reply to Ed Morley [:edmorley] from comment #10)
> Just to help explain the crashes in bug 752168 - this had ended up being
> merged to mozilla-central for 3-4 hours before the backout was also merged
> (we had a 250+ changeset backlog on mozilla-inbound due to CPG, so it
> slipped through with the efforts to lower that as soon as possible), so
> ended up being included in the Nightly respin, which is how crashes were
> occurring in the wild. Today's Nightly will include the backout.

I don't think those crashes are at all related to this bug.
Ah sorry, I was just going by Scoobidiver's added dependency :-)

Updated

5 years ago
No longer depends on: 752168
https://hg.mozilla.org/mozilla-central/rev/79f21117ea59
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
(Assignee)

Comment 14

5 years ago
That was via a mozilla-inbound push that I forgot to note here:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79f21117ea59
(Assignee)

Comment 15

5 years ago
Comment on attachment 620759 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): bug 706193
User impact if declined: font inflation changes when typing in or selecting values in form controls
Testing completed (on m-c, etc.):  on mozilla-central for a bit
Risk to taking this patch (and alternatives if risky): given the mozilla-central testing, reltaively low
String changes made by this patch: none
Attachment #620759 - Flags: approval-mozilla-aurora?
Attachment #620759 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/9b7ed656b15f
status-firefox14: --- → fixed
The test case in comment 1 is fixed the test case in the URL field still grows in size. Tested in Nightly, Aurora and 14 Beta 2.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 757616
In triage I was asked to mark this bug fixed and open a new bug 757616.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 772440
You need to log in before you can comment on or make changes to this bug.