"Take a Screenshot" doesn't work in debug builds
Categories
(Firefox :: Screenshots, defect, P1)
Tracking
()
People
(Reporter: jgilbert, Assigned: emmamalysz)
References
Details
Attachments
(1 file)
|
(deleted),
text/x-phabricator-request
|
Details |
Nothing happens, no warnings in either web/browser consoles.
|
Reporter | |
Comment 1•4 years ago
|
||
True for both +opt and -opt builds, with +debug.
|
Assignee | |
Comment 3•4 years ago
|
||
Hey Jeff,
I have a debug build but can't seem to reproduce (on a mac). I want to clarify: you're not seeing any part of the screenshots UI, right?
I know there were performance concerns when we made screenshots fission compatible (Bug 1664444), but that should really only noticeably impact screenshotting larger images
What happens if you try taking a screenshot via the page action panel and waiting a few seconds? If nothing shows and you open the page action panel again, is the icon greyed out?
|
|
Reporter | |
Comment 4•4 years ago
|
||
I'm on Windows. What's the "page action panel"? I've been right-clicking and hitting "Take screenshot".
|
|
Reporter | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 5•4 years ago
|
||
I was able to reproduce and will start investigating. Thanks for pointing this out!
For reference, the page action panel refers to the panel stemming from the "..." icon in the urlbar. It looks like both routes will reproduce this issue though
|
|
Reporter | |
Comment 6•4 years ago
|
||
Thanks for looking into it!
|
|
Assignee | |
Comment 8•4 years ago
|
||
Hey! Sorry for the delay. I'm trying to find some time to fix it, but I've been able to narrow down what's happening.
We never receive the load event here: https://searchfox.org/mozilla-central/rev/0b2870194375d80b54608c39060884acb758c206/browser/extensions/screenshots/selector/ui.js#104-122. That stems from setting the iframe's source to the blank.html here: https://searchfox.org/mozilla-central/rev/0b2870194375d80b54608c39060884acb758c206/browser/extensions/screenshots/selector/ui.js#72
I had tried setting the src directly to data:text/html,<html></html>, and we then receive the on load event. However, then the this.element.contentDocument would be null, and using this.element.contentWindow.document directly isn't allowed..
Still need to dig in, but if there's something obvious I'm missing, please let me know! I plan on taking a closer look this week
|
|
Assignee | |
Comment 9•4 years ago
|
||
Update: it appears as if browser.extension.getURL is broken. If we change https://searchfox.org/mozilla-central/rev/b9384b091e901b3283ce24b6610e80699d79fd06/browser/extensions/screenshots/selector/ui.js#72 to el.srcdoc = '<html></html>, we receive the load event and open the UI.
However, a lot of the other icons in screenshots rely on this, so it looks pretty broken. Some other tests rely on this, but screenshots looks to be the only component that implements it: https://searchfox.org/mozilla-central/search?q=browser.extension.geturl&path=&case=false®exp=false
|
|
Assignee | |
Comment 10•4 years ago
|
||
|
|
Assignee | |
Comment 11•4 years ago
|
||
The attached patch is a WIP that restores screenshots functionality to debug builds.
However, the icons still appear to be broken, so that needs to be looked into.
|
|
Assignee | |
Comment 12•3 years ago
|
||
I'm having trouble reproducing this issue on a debug build now.. Sam can you reproduce?
|
||
Comment 13•3 years ago
|
||
I'm not able to produce on a (artifact) debug build.
|
|
Assignee | |
Comment 15•3 years ago
|
||
I tried a more recent debug build and still could not reproduce. Niklas, who will be working on screenshots more, also couldn't reproduce.
Is it possible this was fixed?
|
|
Reporter | |
Comment 16•3 years ago
|
||
Yeah, it seems to have fixed itself, though it sometimes takes a (very) long time (>1m) for the UI for it to come up.
|
||
Updated•3 years ago
|
Description
•