Summary: | [Qt] Restore x-position when restoring previous zoom-level | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||
Component: | WebKit2 | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abecsi, menard, webkit.review.bot, zalan, zoltan | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 84602 | ||||||||||||
Attachments: |
|
Description
Allan Sandfeld Jensen
2012-04-23 06:17:02 PDT
Created attachment 138333 [details]
Patch
Comment on attachment 138333 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138333&action=review > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:96 > +static inline bool fuzzyCompare(qreal a, qreal b, qreal f) qFuzzyCompare is not good enough? (In reply to comment #2) > (From update of attachment 138333 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138333&action=review > > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:96 > > +static inline bool fuzzyCompare(qreal a, qreal b, qreal f) > > qFuzzyCompare is not good enough? I tried it, but I still had cases where a double-tap didn't zoom out, and qFuzzyCompare couldn't take an argument for epsilon. I am kinda of surprised I couldn't find a proper Qt floating-point compare with epsilon argument. Created attachment 138355 [details]
Patch
Attachment 138355 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:27: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 138355 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138355&action=review > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:99 > + return std::abs(a - b) < epsilon; I think you could use qAbs() here, then you wouldn't need cmath. Looks good otherwise. (In reply to comment #5) > Attachment 138355 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:27: Alphabetical sorting problem. [build/include_order] [4] > Total errors found: 1 in 3 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. Style-bot is wrong by the way. The includes are sorted with system-includes after other includes. (In reply to comment #7) > (In reply to comment #5) > > Attachment 138355 [details] [details] did not pass style-queue: > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:27: Alphabetical sorting problem. [build/include_order] [4] > > Total errors found: 1 in 3 files > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > Style-bot is wrong by the way. The includes are sorted with system-includes after other includes. I think the bot uses C locale for sort (LC_COLLATE=C) which would expect cmath after QWheelEvent. Created attachment 138361 [details]
Patch
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > Attachment 138355 [details] [details] [details] did not pass style-queue: > > > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 > > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:27: Alphabetical sorting problem. [build/include_order] [4] > > > Total errors found: 1 in 3 files > > > > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > > > Style-bot is wrong by the way. The includes are sorted with system-includes after other includes. > > I think the bot uses C locale for sort (LC_COLLATE=C) which would expect cmath after QWheelEvent. That was where I first placed it. There was no place the style-checker would accept it. Anyway it is gone now, qAbs was a better choice. (In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #7) > > > (In reply to comment #5) > > > > Attachment 138355 [details] [details] [details] [details] did not pass style-queue: > > > > > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 > > > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:27: Alphabetical sorting problem. [build/include_order] [4] > > > > Total errors found: 1 in 3 files > > > > > > > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > > > > > Style-bot is wrong by the way. The includes are sorted with system-includes after other includes. > > > > I think the bot uses C locale for sort (LC_COLLATE=C) which would expect cmath after QWheelEvent. > > That was where I first placed it. There was no place the style-checker would accept it. Anyway it is gone now, qAbs was a better choice. That's probably because your system has something like en_US.UTF-8 as a locale, using LC_COLLATE=C Tools/Scripts/check-webkit-style might give the correct result. Created attachment 138512 [details]
Patch
discussed on #irc. the updated(coming) patch looks good. :) Comment on attachment 138512 [details] Patch Clearing flags on attachment: 138512 Committed r115022: <http://trac.webkit.org/changeset/115022> All reviewed patches have been landed. Closing bug. |