Switch Screenshots from using drawWindow to captureTab, to work with Fission iframes
Categories
(Firefox :: Screenshots, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: zombie, Assigned: emmamalysz)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
Attachments
(1 file)
The canvas drawWindow
method is sync, and thus can't be Fission-compatible for OOP iframes.
In bug 1636508, I introduced additional arguments to the tabs.captureTab
method to specify an area of the document, and the scale (devicePixelRatio). They're not documented yet on MDN, but you should be able to understand them from the schema or tests in the second patch.
Emma or Sam, can you please check if the API addition is enough and appropriate for the Screenshots usecase, ideally in the next week, so that we could document and announce changes to addon devs?
Updated•4 years ago
|
Comment 1•4 years ago
|
||
The only issue I see is the lack of a bgColor
option. Screenshots currently calls drawWindow(window, left, top, width, height, "#fff");
. See shooter.js.
Is a background-color implicit in captureTab
or might we get transparent screenshots without? Otherwise it looks like this should work.
The canvas context we use to call drawWindow is pretty much just used to get a data-uri from. In some cases we also use it to re-encode as JPEG if some capture size threshold is exceeded. We would have to either re-work that or (my preference) remove that step - see https://bugzilla.mozilla.org/show_bug.cgi?id=1649915#c6. - but I don't think that is relevant from the point of view of the extension api.
Reporter | ||
Comment 2•4 years ago
|
||
(In reply to Sam Foster [:sfoster] (he/him) from comment #1)
The only issue I see is the lack of a
bgColor
option. Screenshots currently callsdrawWindow(window, left, top, width, height, "#fff");
. See shooter.js.Is a background-color implicit in
captureTab
or might we get transparent screenshots without? Otherwise it looks like this should work.
Yeah, we have a hard-coded opaque white background, don't think there's need to enable changing that for now:
https://searchfox.org/mozilla-central/rev/f4b4008f5e/toolkit/components/extensions/parent/ext-tabs-base.js#136
The canvas context we use to call drawWindow is pretty much just used to get a data-uri from. In some cases we also use it to re-encode as JPEG if some capture size threshold is exceeded. We would have to either re-work that or (my preference) remove that step - see https://bugzilla.mozilla.org/show_bug.cgi?id=1649915#c6. - but I don't think that is relevant from the point of view of the extension api.
Right, you should be able avoid using a canvas from inside the page at all.
Comment 3•4 years ago
|
||
Hi Emma, this is a Screenshots extension bug that will need to be fixed soon to be compatible with Fission. You can enabled Fission (Site Isolation) in Firefox Nightly's "Nightly Experiments" preferences: about:preferences#experimental
btw, the Module Owner wiki lists you as the triage owner of the "Firefox :: Screenshots" component, but Bugzilla still lists Ian Bicking as the triage owner.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 4•3 years ago
|
||
Pushed by emalysz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/90fd0f5a6bb2 make screenshots fission compatible and remove unused code r=sfoster
Comment 6•3 years ago
|
||
Backed out for turning Bug 1579683 into permafail.
Backout link: https://hg.mozilla.org/integration/autoland/rev/3763cffd1e2ed28738cc9721bf6465bc7410c4a7
Pushed by abutkovits@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1bc4bc380261 make screenshots fission compatible and remove unused code r=sfoster
Comment 8•3 years ago
•
|
||
Relanded this bug: https://hg.mozilla.org/integration/autoland/rev/1bc4bc3802611f3b039078ffcbc3217a9711a912 and sorry for the trouble
Comment 9•3 years ago
|
||
bugherder |
Description
•