Closed
Bug 848760
Opened 10 years ago
Closed 7 years ago
Add a "screenshot" menuitem to the inspector (in the highlighter infobar)
Categories
(DevTools :: Inspector, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1163332
People
(Reporter: paul, Unassigned)
Details
(Whiteboard: [waiting for green try])
Attachments
(2 files, 2 obsolete files)
10.34 KB,
patch
|
Details | Diff | Splinter Review | |
11.93 KB,
patch
|
Details | Diff | Splinter Review |
Comment 1•9 years ago
|
||
Attachment #775134 -
Flags: review?(paul)
Reporter | ||
Comment 2•9 years ago
|
||
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+
Comment 3•9 years ago
|
||
I am just concerned on having three different implementations of an almost similar approach to capture screenshots.
Reporter | ||
Comment 4•9 years ago
|
||
(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.
Comment 5•9 years ago
|
||
(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.
Comment 6•9 years ago
|
||
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).
Reporter | ||
Comment 7•9 years ago
|
||
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.
Comment 8•9 years ago
|
||
Attachment #775134 -
Attachment is obsolete: true
Reporter | ||
Comment 9•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=1e0e3f51ef07
Reporter | ||
Comment 10•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=a1d7ca97ddea
Reporter | ||
Comment 11•9 years ago
|
||
Apparently, there's a test failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=25324149&tree=Try
Reporter | ||
Comment 12•9 years ago
|
||
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.
Reporter | ||
Comment 13•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e37c45c04800
Reporter | ||
Updated•9 years ago
|
Whiteboard: [waiting for green try]
Reporter | ||
Comment 14•9 years ago
|
||
erf, in the test: s/setInterval/setTimeout/
Reporter | ||
Comment 15•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=64ae402cdb7b
Reporter | ||
Comment 16•9 years ago
|
||
Apparently, the test (or the feature) doesn't work on Windows XP.
Comment 17•9 years ago
|
||
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
Reporter | ||
Comment 18•9 years ago
|
||
Because the inspector now works as a remote tool, I needed to move this code at the actor level.
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1163332
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•