Summary: | [EFL][GTK][Qt] New test: fast/regions/autoheight-correct-region-for-lines-2.html fails after r152572. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gábor Ábrahám <abrhm> | ||||||||
Component: | Tools / Tests | Assignee: | Javier Fernandez <jfernandez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abucur, dino, jfernandez, kadam, mcatanzaro, mihnea, mrobinson, ossy, rniwa, simon.fraser, spenap, thorton, WebkitBugTracker, zan, zarvai | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 119273 | ||||||||||
Bug Blocks: | 63344, 87008, 118531 | ||||||||||
Attachments: |
|
Description
Gábor Ábrahám
2013-07-12 04:33:52 PDT
Created attachment 206538 [details]
The actual picture
Created attachment 206541 [details]
Actual
I will skip this test on Qt now, and upload the actual picture here, finally.
Thanks for the report, we'll take a look at it. I can work on this bug, if nobody tells me otherwise. After looking into this issue for a while, I've found out that the issue is related to an arithmetic overflow when computing the logic height. The maxHeight property is defined in the test as a huge value, which is handled as a Length instance based on a float type. I've found out that the flag SATURATED_LAYOUT_ARITHMETIC is not enabled by default in the WebKitGtk+ port, so when calling to the LayoutUnit(float), it does not apply the clampTo<float> template function. Just enabling the SATURATED_LAYOUT_ARITHMETIC flag makes the test to pass, at least for the WebkitGtk+ port, but I'm still doing some further investigations to understand why this is happening, whether is a Region related issue or a more general issue, Besides, I want to understand how the combination of both, SATURATED_LAYOUT_ARITHMETIC and SUBPIXEL_LAYOUT actually works and which cases each of them handle. This is clearly a bug not related to Regions at all, since the actual issue was reported in the bug #119273. A simple fix would be just to set a lower value for the max-height property (like 20000000px, big enough, I would say). Of course it's also possible to wait for the bug #119273 fix, but notice that the max-height value has noting to do with the purpose of the test, besides allowing the region to adjust its auto-height properly. Created attachment 210642 [details]
Patch
Comment on attachment 210642 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=210642&action=review > LayoutTests/fast/regions/autoheight-correct-region-for-lines-2.html:20 > - max-height: 2000000000px; > + max-height: 20000000px; I'm not sure if it's such a good idea to workaround this bug in the test. This test was valuable in that it caught one case in which enabling subpixel layout without saturated arithmetic would result in a bug. Do we have a test in LayoutTests/fast/sub-pixel that catches the same bug? If not, please add one if you decide to "fix" the test. (In reply to comment #9) > (From update of attachment 210642 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=210642&action=review > > > LayoutTests/fast/regions/autoheight-correct-region-for-lines-2.html:20 > > - max-height: 2000000000px; > > + max-height: 20000000px; > > I'm not sure if it's such a good idea to workaround this bug in the test. > This test was valuable in that it caught one case in which enabling subpixel layout without saturated arithmetic would result in a bug. > Do we have a test in LayoutTests/fast/sub-pixel that catches the same bug? If not, please add one if you decide to "fix" the test. I share the concerns as well, but on the other hand, that was not the purpose of the tests. I've filed a new one specifically for this issue at bug #119273, which also provides the corresponding test. My idea would be to close this bug and track the arithmetic overflow issue in the specific bug. Besides, the fix for bug #119273 would imply, as discussed in the mailing list, merging the 2 flags so that SATURATED_LAYOUT_ARITHMETIC would be enabled whenever SUBPIXEL_LAYOUT is. Anyway, I've been talking about this with Mihnea Ovidenie so perhaps we should wait to get some feedback from him. Also, we could wait until bug #119273 is fixed to take the final decision. I'm simply concerned about losing the test coverage even for temporarily. This bug should be fixed now, at least in the WebKitGtk+ port, now that the SATURATED_LAYOUT_ARITHMETIC is enabled (bug 119273). Comment on attachment 210642 [details]
Patch
We no longer have failure expectations for this bug, so I guess it is obsolete.
|