| Summary: | [Qt] Mini clean ups in the interaction engine | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kenneth Rohde Christiansen <kenneth> | ||||
| Component: | WebKit Qt | Assignee: | Kenneth Rohde Christiansen <kenneth> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | hausmann, menard, webkit.review.bot, zalan, zoltan | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Kenneth Rohde Christiansen
2012-05-21 06:15:36 PDT
Created attachment 143017 [details]
Patch
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 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 Landed in 117952 Sure Zalan will run into the assert :-) that is why we add them! (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. |