Summary: | Web Inspector: Canvas: recording parameters that include an Image should show an InlineSwatch (2D canvas) | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||||||||||||||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | bburg, buildbot, commit-queue, inspector-bugzilla-changes, mattbaker, rniwa, webkit-bug-importer | ||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||
Bug Blocks: | 173807, 179207 | ||||||||||||||||||||||||
Attachments: |
|
Description
Devin Rousso
2017-09-15 22:54:42 PDT
It could be good to have a popover with larger thumbnail when clicking/hovering the swatch. Created attachment 320996 [details]
Patch
Created attachment 320997 [details]
[Image] After Patch is applied
Created attachment 320998 [details]
Patch
Add swatches for CanvasGradient/CanvasPattern in the DetailsSidebar
Created attachment 320999 [details]
[Image] After Patch is applied
Created attachment 324966 [details]
Patch
Rebased.
Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4995206 New failing tests: inspector/canvas/recording-2d.html Created attachment 324971 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4995090 New failing tests: inspector/canvas/recording-2d.html Created attachment 324972 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 324966 [details] Patch Attachment 324966 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4998205 New failing tests: inspector/canvas/recording-2d.html Created attachment 325008 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 325056 [details]
Patch
Added missing include for tests
Comment on attachment 325056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325056&action=review r=me, nice work. > Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:59 > + WI.scratchCanvasContext2D = function(callback) { This is clever, but I don't like adding new functions to the top level namespace. I'd prefer a more boring design, like extracting this to a WI.ImageRenderer or WI.ImageUtilities class with static methods. Then if for some reason we need multiple scratchpads it is trivial to do so. let image = WI.ImageUtilities.imageFromImageData(data) let image = WI.ImageUtilities.imageFromScratchCanvas((context) => ...) > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:144 > + let fragment = document.createDocumentFragment(); Nit: move closer to the use site. > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:152 > + Nit: extra space > Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:156 > + Ditto. Comment on attachment 325056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325056&action=review >> Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:59 >> + WI.scratchCanvasContext2D = function(callback) { > > This is clever, but I don't like adding new functions to the top level namespace. I'd prefer a more boring design, like extracting this to a WI.ImageRenderer or WI.ImageUtilities class with static methods. Then if for some reason we need multiple scratchpads it is trivial to do so. > > let image = WI.ImageUtilities.imageFromImageData(data) > let image = WI.ImageUtilities.imageFromScratchCanvas((context) => ...) Will address in followup to avoid making this more complex. <https://webkit.org/b/179207> Created attachment 325787 [details]
Patch
Comment on attachment 325787 [details] Patch Clearing flags on attachment: 325787 Committed r224367: <https://trac.webkit.org/changeset/224367> All reviewed patches have been landed. Closing bug. |