RESOLVED FIXED 100912
Fix assertion fail in RenderGeometryMap::absoluteRect when frame scale != 1.0
https://bugs.webkit.org/show_bug.cgi?id=100912
Summary Fix assertion fail in RenderGeometryMap::absoluteRect when frame scale != 1.0
Tien-Ren Chen
Reported 2012-10-31 19:04:58 PDT
Fix assertion fail in RenderGeometryMap::absoluteRect when frame scale != 1.0
Attachments
Patch (5.76 KB, patch)
2012-10-31 19:08 PDT, Tien-Ren Chen
no flags
Patch (4.92 KB, patch)
2012-11-01 00:05 PDT, Tien-Ren Chen
no flags
Patch (5.58 KB, patch)
2012-11-01 13:31 PDT, Tien-Ren Chen
no flags
Patch (4.92 KB, patch)
2012-11-01 13:53 PDT, Tien-Ren Chen
no flags
Patch (5.19 KB, patch)
2012-11-01 14:15 PDT, Tien-Ren Chen
no flags
Tien-Ren Chen
Comment 1 2012-10-31 19:08:37 PDT
Simon Fraser (smfr)
Comment 2 2012-10-31 20:10:31 PDT
Comment on attachment 171758 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171758&action=review > Source/WebCore/rendering/RenderGeometryMap.cpp:125 > + // If this box has a transform, it acts as a fixed position container > + // for fixed descendants, which prevents the propagation of 'fixed' > + // unless the layer itself is also fixed position. > + if (currentStep.m_hasTransform && !currentStep.m_isFixedPosition) > + inFixed = false; > + else if (currentStep.m_isFixedPosition) > + inFixed = true; I'm not sure that moving this is correct. I think we may just want to special case the RenderView (step 0) to not disable the fixed handling if it has a transform. Do we have test coverage for fixed inside transformed? > LayoutTests/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=100912 fail -> failure > LayoutTests/ChangeLog:12 > + tranformation case. tranformation -> transformation
Tien-Ren Chen
Comment 3 2012-11-01 00:05:04 PDT
Levi Weintraub
Comment 4 2012-11-01 04:48:21 PDT
Comment on attachment 171783 [details] Patch Great catch! I've hit that assert many times.
WebKit Review Bot
Comment 5 2012-11-01 05:38:36 PDT
Comment on attachment 171783 [details] Patch Rejecting attachment 171783 [details] from commit-queue. New failing tests: compositing/geometry/fixed-position-composited-page-scale-scroll.html platform/chromium/virtual/softwarecompositing/geometry/fixed-position-composited-page-scale-scroll.html Full output: http://queues.webkit.org/results/14696313
WebKit Review Bot
Comment 6 2012-11-01 05:59:28 PDT
Comment on attachment 171783 [details] Patch Attachment 171783 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14661592 New failing tests: compositing/geometry/fixed-position-composited-page-scale-scroll.html platform/chromium/virtual/softwarecompositing/geometry/fixed-position-composited-page-scale-scroll.html
Tien-Ren Chen
Comment 7 2012-11-01 13:31:18 PDT
Tien-Ren Chen
Comment 8 2012-11-01 13:31:53 PDT
Change to ref-test
Simon Fraser (smfr)
Comment 9 2012-11-01 13:39:29 PDT
Comment on attachment 171918 [details] Patch I don't see the point of having a ref test for an assertion failure. Ref tests are slower to run.
Tien-Ren Chen
Comment 10 2012-11-01 13:53:42 PDT
Simon Fraser (smfr)
Comment 11 2012-11-01 14:10:45 PDT
Comment on attachment 171920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171920&action=review > LayoutTests/compositing/geometry/fixed-position-composited-page-scale-scroll-expected.txt:2 > +TEST > +TEST It's useful for the test output to say what the test is testing. So something like "This test should not hit an assertion in RenderGeometryMap in debug builds".
Tien-Ren Chen
Comment 12 2012-11-01 14:15:18 PDT
WebKit Review Bot
Comment 13 2012-11-01 18:19:23 PDT
Comment on attachment 171929 [details] Patch Clearing flags on attachment: 171929 Committed r133248: <http://trac.webkit.org/changeset/133248>
WebKit Review Bot
Comment 14 2012-11-01 18:19:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.