Closed Bug 1447043 Opened 2 years ago Closed 2 years ago

Selection UI positioning errors in latest Screenshots

Categories

(Firefox :: Screenshots, defect)

x86_64
All
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- verified
firefox61 --- verified

People

(Reporter: _6a68, Assigned: _6a68)

Details

Attachments

(2 files, 1 obsolete file)

Attachment #8960249 - Attachment is obsolete: true
Attachment #8960249 - Flags: review?(jhirsch)
Comment on attachment 8960250 [details]
Bug 1447043 - Export Screenshots 30.1.0 to Firefox;

https://reviewboard.mozilla.org/r/229006/#review234754
Attachment #8960250 - Flags: review?(ianb) → review+
Comment on attachment 8960254 [details]
Bug 1447043 - Calculate top and left selection margin using left and top;

https://reviewboard.mozilla.org/r/229016/#review234752
Attachment #8960254 - Flags: review?(jhirsch) → review+
Pushed by jhirsch@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cc8d54d3b19c
Calculate top and left selection margin using left and top; r=_6a68
https://hg.mozilla.org/integration/autoland/rev/1db369ac3b20
Export Screenshots 30.1.0 to Firefox; r=ianbicking
Assignee: nobody → jhirsch
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
I have retested this issue on latest Nightly (61.0a1, build from 2018-03-21) build and the issue is no longer reproducible. The Screenshots selection is working as expected on the reported websites:
- https://www.ghacks.net/2018/03/12/firefox-59-firefox-screenshots-gets-image-editing-functionality/
- https://www.reddit.com/r/iamverysmart/comments/84b7dn/message_for_all_the_people_who_think_iq_matters/
- https://brightside.me/
- https://www.npr.org/

I can also confirm that the issue is reproducible on older Nightly (eg: build from 2018-03-19) builds. In order to reproduce issue I have followed the provided STR from the GitHub issue: https://github.com/mozilla-services/screenshots/issues/4217

In order to make sure if there are no new issues or regressions, I have verified if the Screenshots selection works as expected on the first 50 websites from https://www.alexa.com/topsites page and also a few more that we consider popular (eg: cnn.com, nytimes.com, stackoverflow.com, imdb.com, pinterest.com etc). During testing I haven't found any new issues.

Tested this on Windows 7 x64, Mac 10.13 and Arch Linux.
OS: Unspecified → All
Hardware: Unspecified → x86_64
Comment on attachment 8960250 [details]
Bug 1447043 - Export Screenshots 30.1.0 to Firefox;

Approval Request Comment
[Feature/Bug causing the regression]: On a few websites, when the user starts the screenshot flow by clicking to select a region, the selection will be drawn in the wrong place, sometimes off screen. For more details, see [1], [2], [3].
[User impact if declined]: Shot creation flow will seem broken in some cases. 
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, already tested by SV.
[List of other uplifts needed for the feature/fix]: Both patches attached to this bug (one to fix the bug, one to increment the addon version number).
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: The patch is very small, is confined to webextension code, and only affects the selection step in the screenshot creation flow.
[String changes made/needed]: None


[1] https://github.com/mozilla-services/screenshots/issues/4217
[2] https://github.com/mozilla-services/screenshots/issues/3907
[3] https://github.com/mozilla-services/screenshots/issues/3972
Attachment #8960250 - Flags: approval-mozilla-beta?
Target Milestone: --- → Firefox 61
Comment on attachment 8960250 [details]
Bug 1447043 - Export Screenshots 30.1.0 to Firefox;

screenshots bug fix, beta60+
Attachment #8960250 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Firefox	60.0b6, Build ID: 20180322152034

I can confirm that the issue is fixed in latest Beta 60.0b6 build. Tested this on Windows 10 x64, Windows 7 x64, Mac 10.12 and Ubuntu 14.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.