Closed Bug 848760 Opened 7 years ago Closed 4 years ago
Add a "screenshot" menuitem to the inspector (in the highlighter infobar)
Comment on attachment 775134 [details] [diff] [review] Add screenshot menu item to inspector Looks good to me! Thanks. Just rename panelWindow to browserWindow.
Attachment #775134 - Flags: review?(paul) → review+
I am just concerned on having three different implementations of an almost similar approach to capture screenshots.
(In reply to Girish Sharma [:Optimizer] from comment #3) > I am just concerned on having three different implementations of an almost > similar approach to capture screenshots. The one from gcli is much more complex (because it does more things) and barely reachable. I will rebase bug 849236 on this, and move the screenshot function to LayoutHelpers.
(In reply to Paul Rouget [:paul] from comment #4) > The one from gcli is much more complex (because it does more things) and > barely reachable. I will rebase bug 849236 on this, and move the screenshot > function to LayoutHelpers. Great! Sounds reasonable. I know that the one in gcli is much complex, but that will be just appropriate in this case, where we want to take a screenshot of a dom element (as that one already does that). Although, it would have been nice if the method used in gcli could be moved to layout helpers and used everywhere.
Should the method in GCLI splitted into two at this point (move the first part to layout helper)? http://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/BuiltinCommands.jsm#1713 Maybe another one to generate the auto generated filename as well (If they should use the same name pattern).
Let's keep things simple and land this patch. I will move things to LayoutHelpers later. Please rename the variable and I'll land that.
Apparently, there's a test failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=25324149&tree=Try
Not sure why it failed here... I might have pushed the wrong patch, and the previous push is not accessible anymore. I'll re-push to try.
erf, in the test: s/setInterval/setTimeout/
Apparently, the test (or the feature) doesn't work on Windows XP.
Updated the test case to use setTimeout. I tried building and testing in XP. Even though the screenshot is saved to Download folder, the test case treat DfltDwnId as \Desktop. If I copied the screenshot to desktop before running the test case, the test will success and the desktop file is deleted.
Attachment #775888 - Attachment is obsolete: true
Because the inspector now works as a remote tool, I needed to move this code at the actor level.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1163332
You need to log in before you can comment on or make changes to this bug.