Closed Bug 410405 Opened 17 years ago Closed 17 years ago

textboxes size="xy" is not respected

Categories

(Core :: Graphics, defect, P2)

Product:
Core
Component:
Graphics
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9

People

(Reporter: lorenzo.fiorini, Assigned: karlt)

References

Details

(Keywords: regression, Whiteboard: [swag:1d])

Attachments

(5 files, 1 obsolete file)

FF3b2 screenshot
(deleted), image/png
Details
FF2 screenshot
(deleted), image/png
Details
testcase
(deleted), text/html
Details
consider pango_font_metrics_get_approximate_digit_width() in aveCharWidth [checked-in]
(deleted), patch
pavlov
: review+
mtschrep
: approval1.9+
Details | Diff | Splinter Review
wrap rightmost textboxes in text_textbox_number so that mouse events are tested closer to the left hand side of the frame [checked-in]
(deleted), patch
pavlov
: review+
mtschrep
: approval1.9+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2 The number of chars shown in FF3b2 is smaller than requested even if a monospace font is used. FF2 works. Reproducible: Always Steps to Reproduce: 1. Save the html below into a file 2. Open it with FF2 and FF3b2 3. <html> <head> <title>Example</title> </head> <style> .mytextbox { font-family: monospace !important; } </style> <body> <div class="mytextbox"> <form> <table> <tr> <td colspan="3"> 12345678901234567890123456789012345:<br/> <input type="text" id="name" maxlength="35" size="35"/> </td> </tr> <tr> <td colspan="3"> 12345678901234567890123456789012345:<br/> <input type="text" id="surname" maxlength="35" size="35"/> </td> </tr> <tr> <td colspan="3"> 123456789012345678901234567890123456789012345:</br> <input type="text" id="address" maxlength="45" size="45"/><br/> </td> </tr> <tr> <td> 12345:<br/> <input type="text" id="zip" maxlength="5" size="5"/><br/> </td> <td> 12345678901234567890:<br/> <input type="text" id="city" maxlength="20" size="20"/><br/> </td> <td> 12:<br/> <input type="text" id="state" maxlength="2" size="2"/><br/> </td> </tr> </table> </form> </div> </body> </html> Actual Results: FF3b2 shows less chars than the requested size Expected Results: textboxes should show the chars requested in size or at least works as FF2
Attached image FF3b2 screenshot (deleted) —
Attached image FF2 screenshot (deleted) —
This is a regression from bug 362682. Note that this is font dependent: with a monospace font, the input is wide enough to show all characters.
Blocks: 362682
Status: UNCONFIRMED → NEW
Component: General → GFX: Thebes
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
QA Contact: general → thebes
Version: unspecified → Trunk
Attached file testcase (deleted) —
This has to do with font metrics. When using a sans font (DejaVuSans.ttf), aveCharWidth is 6.3076171875 at default font size (13.3333px). On branch (I didn't have a pre bug 362682 build to test), this value is 8. For monospace font, the aveCharWidth is equal to maxAdvance, which is 8 on my system (font size of 13.3333px). This is the same on branch. So apparently the value returned from pango_font_metrics_get_approximate_char_width is a bit too small when computing the width of an input element with a non monospace font.
(In reply to comment #5) > This has to do with font metrics. When using a sans font (DejaVuSans.ttf), > aveCharWidth is 6.3076171875 at default font size (13.3333px). > > On branch (I didn't have a pre bug 362682 build to test), this value is 8. > > For monospace font, the aveCharWidth is equal to maxAdvance, which is 8 on my > system (font size of 13.3333px). This is the same on branch. > > So apparently the value returned from > pango_font_metrics_get_approximate_char_width is a bit too small when computing > the width of an input element with a non monospace font. > I confirm, using monospace in input gives the right width. If can help I can say that using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007092115 Webrunner/0.7 it worked.
This can be very inconvenient when trying to enter credit card numbers. Both Mac and Windows are using the width of an "x", which seems to be where the metric was originally derived from. http://msdn2.microsoft.com/en-us/library/ms534202.aspx Linux used to use the width of an "x" but this changed in bug 362682 to pango_font_metrics_get_approximate_char_width(), which is language dependent. The language dependent version sounds like a good idea for non-roman fonts, but it seems that the width of an "x" is usually slightly larger than an "average" width. We're already measuring an "x" for height so we could easily take the maximum of "average" and x-width. We could also include pango_font_metrics_get_approximate_digit_width() in the computation as this has already been calculated for us.
Flags: blocking1.9?
Priority: -- → P3
--> karlt
Assignee: nobody → mozbugz
Flags: blocking1.9? → blocking1.9+
Priority: P3 → P2
Whiteboard: [swag:1d]
Flags: tracking1.9+
Use a maximum of pango_font_metrics_get_approximate_char_width and pango_font_metrics_get_approximate_digit_width for the aveCharWidth metric. The maximum numeral (0-9) width has already been calculated. pango_font_metrics_get_approximate_digit_width just fetches the result. (And there is no longer any need for testing mAdjustedSize against 0.)
Attachment #314165 - Flags: review?(pavlov)
Attachment #314165 - Flags: review?(pavlov) → review+
I'm not so keen on a test with variable width fonts as there is no defined variable width behavior, but we could/should make sure there is a test with monospace font.
Flags: in-testsuite?
Backed out due to failure on reftests/bugs/376532-1.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The reftest makes an assumption about the aspect ratio of em height to missing glyph hex box width (which is >= aveCharWidth changed here). I've updated the reftest to loosen the assumption a little. http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1207724165&maxdate=1207724331&who=karlt%2B%25karlt.net (still need to reland this.)
attachment 314165 [details] [diff] [review] also caused toolkit/content/tests/widgets/test_textbox_number.xul to fail to receive mouse events on the spin buttons on the right-most number textboxes. I can reproduce the same results by using a small enough window. It seems that either this mochitest should be updated to not be so sensitive to font character widths or the width of the window for the tests should be wider.
(In reply to comment #16) > attachment 314165 [details] [diff] [review] also caused > toolkit/content/tests/widgets/test_textbox_number.xul to fail to receive mouse > events on the spin buttons on the right-most number textboxes. I can reproduce > the same results by using a small enough window. > > It seems that either this mochitest should be updated to not be so sensitive to > font character widths or the width of the window for the tests should be wider. > Can you either mod the test or talk to DBaron/Roc about it?
Comment on attachment 314779 [details] [diff] [review] swap n2 and n3 in text_textbox_number so that mouse events are tested closer to the left hand side of the frame It would be better to reduce the size of the textboxes or increase the width of the frame. Recently I increased the height of the frame (testing/mochitest/server.js) so there may also be more room vertically now to put the textboxes on another row. I'd rather do either of those than adjust the test in some way.
Attachment #314779 - Flags: review?(enndeakin) → review-
(In reply to comment #19) > (From update of attachment 314779 [details] [diff] [review]) > ... so there may also be more room vertically now to > put the textboxes on another row. > > I'd rather do either of those than adjust the test in some way. I didn't see any reason why the order of the textboxes is significant, but I've put the rightmost textboxes onto extra rows instead of swapping the order of elements. This would mean that it is more likely that all textboxes would be within the frame. I've also made the textboxes that were on the right the same width as the other leftmost boxes, just to make the visual appearance more pleasing.
Attachment #314779 - Attachment is obsolete: true
Attachment #314999 - Flags: review?(enndeakin)
(In reply to comment #13) > we could/should make sure there is a test with monospace font. There were no such tests so there are now some: forms/input-text-size-*.html (and bug 428458 is reported).
Comment on attachment 314999 [details] [diff] [review] wrap rightmost textboxes in text_textbox_number so that mouse events are tested closer to the left hand side of the frame [checked-in] go ahead and get this in. if enn has issues, we can check in a new patch for the text
Attachment #314999 - Flags: review?(enndeakin) → review+
Attachment #314165 - Flags: approval1.9+
Attachment #314999 - Flags: approval1.9+
Comment on attachment 314999 [details] [diff] [review] wrap rightmost textboxes in text_textbox_number so that mouse events are tested closer to the left hand side of the frame [checked-in] http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1208301420&maxdate=1208301482&who=karlt%2B%25karlt.net
Attachment #314999 - Attachment description: wrap rightmost textboxes in text_textbox_number so that mouse events are tested closer to the left hand side of the frame → wrap rightmost textboxes in text_textbox_number so that mouse events are tested closer to the left hand side of the frame [checked-in]
Comment on attachment 314165 [details] [diff] [review] consider pango_font_metrics_get_approximate_digit_width() in aveCharWidth [checked-in] http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1208307634&maxdate=1208307781&who=karlt%2B%25karlt.net
Attachment #314165 - Attachment description: consider pango_font_metrics_get_approximate_digit_width() in aveCharWidth → consider pango_font_metrics_get_approximate_digit_width() in aveCharWidth [checked-in]
(In reply to comment #13) > I'm not so keen on a test with variable width fonts as there is no defined > variable width behavior. But we could still add reftests so that we notice if the current behavior changes.
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: