|Summary:||[Qt] Mini clean ups in the interaction engine|
|Product:||WebKit||Reporter:||Kenneth Rohde Christiansen <kenneth>|
|Component:||WebKit Qt||Assignee:||Kenneth Rohde Christiansen <kenneth>|
|Severity:||Normal||CC:||hausmann, menard, webkit.review.bot, zalan, zoltan|
|Version:||528+ (Nightly build)|
Description Kenneth Rohde Christiansen 2012-05-21 06:15:36 PDT
Comment 1 Kenneth Rohde Christiansen 2012-05-21 06:16:51 PDT
Created attachment 143017 [details] Patch
Comment 2 zalan 2012-05-21 07:13:56 PDT
a little bit unrelated but since you are touching animateItemRectVisible(): it looks like after a few refactorings, it does not need to be public method anymore (neither it's pair setItemRectVisible()) Could you commit them as private, unless there's a good reason to still have them around as public?
Comment 3 Simon Hausmann 2012-05-22 02:43:39 PDT
Comment on attachment 143017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143017&action=review I'll have to trust you on the logic and welcome any change that adds test coverage to those magic if statements ;-) > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:191 > + ASSERT(m_suspended); I bet Zalan is going to run into this one ;) > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:618 > + * FIXME: This is currently called twise if you concurrently change width and height. twise -> twice
Comment 4 Kenneth Rohde Christiansen 2012-05-22 04:55:00 PDT
Landed in 117952 Sure Zalan will run into the assert :-) that is why we add them!
Comment 5 zalan 2012-05-22 05:29:28 PDT
(In reply to comment #4) > Landed in 117952 > > Sure Zalan will run into the assert :-) that is why we add them! and i am so grateful for that.