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)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: botond, Assigned: kats)

References

Details

(Whiteboard: gfx-noted)

Attachments

(4 files)

      No description provided.
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
Whiteboard: gfx-noted
OS: Gonk (Firefox OS) → Android
Priority: -- → P3
I'm not actively working on this.
Assignee: botond → nobody
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
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+
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+
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+
(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.
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: