Closed
Bug 1188955
Opened 9 years ago
Closed 6 years ago
Write mochitests for double-tap-to-zoom functionality
Categories
(Core :: Panning and Zooming, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: botond, Assigned: kats)
References
Details
(Whiteboard: gfx-noted)
Attachments
(4 files)
No description provided.
Reporter | ||
Comment 1•9 years ago
|
||
I'd like to do this as a follow-up to bug 1131359, where we ported the double-tap-to-zoom implementation to C++.
Depends on: 1131359
Comment 2•9 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Whiteboard: gfx-noted
Assignee | ||
Updated•8 years ago
|
OS: Gonk (Firefox OS) → Android
Assignee | ||
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 4•6 years ago
|
||
I wrote a basic double-tap-to-zoom mochitest that works locally (android x86 emulator). https://treeherder.mozilla.org/#/jobs?repo=try&revision=2da29f632da3be501be4f1213e9a38be74b648d8 It would have helped me catch an error in my patches for bug 1445662.
Assignee: nobody → bugmail
Assignee | ||
Comment 5•6 years ago
|
||
Now with more robustness: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8983b11ed15c8f9a19f357a249a8c5d03ada266b
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8959288 [details] Bug 1188955 - Use the subtest's ok/is functions so the subtest name shows up in the log. https://reviewboard.mozilla.org/r/228148/#review234852
Attachment #8959288 -
Flags: review?(botond) → review+
Reporter | ||
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8959289 [details] Bug 1188955 - Turn the MAX_TAP_TIME into a preffable value. https://reviewboard.mozilla.org/r/228150/#review234854 ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:317 (Diff revision 1) > * keyboard APZ disabled. > * > + * \li\b apz.max_tap_time > + * Maximum time for a touch on the screen and corresponding lift of the finger > + * to be considered a tap. This also applies to double taps, except that it is > + * used both for for the interval between the first touchdown and first touchup, duplicate "for" ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:319 (Diff revision 1) > + * \li\b apz.max_tap_time > + * Maximum time for a touch on the screen and corresponding lift of the finger > + * to be considered a tap. This also applies to double taps, except that it is > + * used both for for the interval between the first touchdown and first touchup, > + * and for the interval between the first touchup and the second touchdown.\n > + * Units: millseconds. typo: "millseconds"
Attachment #8959289 -
Flags: review?(botond) → review+
Reporter | ||
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8959290 [details] Bug 1188955 - Add a basic double-tap-to-zoom mochitest. https://reviewboard.mozilla.org/r/228152/#review234870 Thanks! (And sorry for sitting on this bug and not fixing it for so long.) ::: gfx/layers/apz/test/mochitest/helper_basic_doubletap_zoom.html:58 (Diff revision 1) > +</style> > +</head> > +<body> > +<div class="box">Text before the div.</div> > +<div id="target" style="width:400px"> > +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean ante tellus, laoreet fermentum mattis at, ullamcorper ac turpis. Curabitur porta accumsan nisi eget rhoncus. Integer ultrices interdum porttitor. Duis varius rhoncus leo, vitae consequat odio feugiat nec. Praesent sed tincidunt sapien. Nulla porttitor, sapien ut pulvinar sollicitudin, velit nisl sodales turpis, quis mattis lacus risus sed est. Nulla hendrerit commodo porta. Aliquam erat volutpat. In semper elit ac lacus interdum nec rhoncus nunc porttitor. Suspendisse tincidunt molestie lorem, vel convallis sem convallis nec. Quisque a varius est. Donec id tristique nunc. Donec iaculis justo ac nibh blandit vel placerat leo feugiat. Proin rhoncus lacinia felis id aliquet. Morbi volutpat malesuada enim, sed mollis purus lacinia ac. I know I used to write tests like this, but as my CSS skills improved and I discovered that you can make things take a lot of space on the page without taking up a lot of space in the source (e.g. using gradients), I've come to prefer that. Could we do something like that instead?
Attachment #8959290 -
Flags: review?(botond) → review+
Assignee | ||
Comment 12•6 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #10) > > + * used both for for the interval between the first touchdown and first touchup, > > duplicate "for" Fixed > ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:319 > > + * Units: millseconds. > > typo: "millseconds" Fixed (In reply to Botond Ballo [:botond] from comment #11) > I know I used to write tests like this, but as my CSS skills improved and I > discovered that you can make things take a lot of space on the page without > taking up a lot of space in the source (e.g. using gradients), I've come to > prefer that. Could we do something like that instead? Good point, done. My mistake was starting with the test files Randall attached :) I'll re-run the test locally to make sure it still passes with the gradient before landing.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•6 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b9cb68851e9b Use the subtest's ok/is functions so the subtest name shows up in the log. r=botond https://hg.mozilla.org/integration/autoland/rev/814f27572ced Turn the MAX_TAP_TIME into a preffable value. r=botond https://hg.mozilla.org/integration/autoland/rev/5ed5a088c782 Add a basic double-tap-to-zoom mochitest. r=botond
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b9cb68851e9b https://hg.mozilla.org/mozilla-central/rev/814f27572ced https://hg.mozilla.org/mozilla-central/rev/5ed5a088c782
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
status-firefox42:
affected → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•