When zooming out on tap to zoom the previous zoom-level is reestablished, but the previous position is not. The X-position needs to be restored to ensure the columns that were visible before zooming-in will be visible again after zooming out. Y-position does not need to be restored since zooming is fit-to-width, and moving Y-position is an intended feature of double-tap.
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.