Bug 96673 - Text Autosizing: Add explanations to tests.
Summary: Text Autosizing: Add explanations to tests.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Mellor
URL:
Keywords:
Depends on:
Blocks: FontBoosting
  Show dependency treegraph
 
Reported: 2012-09-13 11:01 PDT by John Mellor
Modified: 2012-09-13 13:22 PDT (History)
8 users (show)

See Also:


Attachments
Patch (28.52 KB, patch)
2012-09-13 11:04 PDT, John Mellor
no flags Details | Formatted Diff | Diff
Patch (27.15 KB, patch)
2012-09-13 12:54 PDT, John Mellor
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Mellor 2012-09-13 11:01:43 PDT
Text Autosizing: Add explanations to tests.
Comment 1 John Mellor 2012-09-13 11:04:47 PDT
Created attachment 163911 [details]
Patch
Comment 2 Adam Barth 2012-09-13 11:07:27 PDT
Comment on attachment 163911 [details]
Patch

rs=me
Comment 3 Julien Chaffraix 2012-09-13 11:13:38 PDT
Comment on attachment 163911 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163911&action=review

> LayoutTests/fast/text-autosizing/font-scale-factor-expected.html:6
> +<style type="text/css">

The 'type' attribute is not required.

> LayoutTests/fast/text-autosizing/font-scale-factor-expected.html:15
> +    This text should be autosized to 80px computed font-size (16 * 800/320 * 2, where 2 is the fontScaleFactor). Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sapien sapien, tempus id egestas a, consectetur et felis. Ut pharetra mi eget lectus laoreet rutrum. Mauris tellus odio, egestas vitae sodales in, vehicula eget ante. Pellentesque id egestas arcu. Quisque tellus quam, rhoncus ac elementum vel, volutpat vel felis. Aliquam ut leo dolor, eget egestas tellus. Nunc ut velit gravida nisl fringilla rutrum eget at arcu. Vivamus et pretium mauris.

Putting the explanation on its own line would make the text more readable (not repeated on all the tests).

> LayoutTests/fast/text-autosizing/narrow-iframe-expected.html:7
> -body { width: 800px; margin: 0; overflow-y: hidden; }
> +body { width: 800px; margin: 0; }

If you remove those, you will check that we don't overflow in those tests. Not sure if it's super useful as you set the height to be tall enough anyway.

> LayoutTests/fast/text-autosizing/narrow-iframe.html:7
> -body { width: 800px; margin: 0; overflow-y: hidden; }
> +body { width: 800px; margin: 0; }

Ditto.
Comment 4 John Mellor 2012-09-13 12:54:08 PDT
Created attachment 163942 [details]
Patch
Comment 5 Julien Chaffraix 2012-09-13 12:59:36 PDT
Comment on attachment 163942 [details]
Patch

r=me, thanks for making the tests more awesome.
Comment 6 WebKit Review Bot 2012-09-13 13:22:51 PDT
Comment on attachment 163942 [details]
Patch

Clearing flags on attachment: 163942

Committed r128500: <http://trac.webkit.org/changeset/128500>
Comment 7 WebKit Review Bot 2012-09-13 13:22:54 PDT
All reviewed patches have been landed.  Closing bug.