RESOLVED FIXED Bug 103752
Web Inspector: [Overrides] Add ability to override the page CSS media type
https://bugs.webkit.org/show_bug.cgi?id=103752
Summary Web Inspector: [Overrides] Add ability to override the page CSS media type
Alexander Pavlov (apavlov)
Reported 2012-11-30 09:13:12 PST
It would be great to have an ability switching media type of the page to print in the DevTools to see what CSS get applied and the result html. Upstreaming http://code.google.com/p/chromium/issues/detail?id=161496
Attachments
Patch (16.07 KB, patch)
2012-11-30 09:29 PST, Alexander Pavlov (apavlov)
no flags
Patch (19.03 KB, patch)
2012-12-03 01:04 PST, Alexander Pavlov (apavlov)
no flags
Patch (19.16 KB, patch)
2012-12-03 01:57 PST, Alexander Pavlov (apavlov)
no flags
Patch for landing (20.15 KB, patch)
2012-12-04 01:59 PST, Alexander Pavlov (apavlov)
webkit.review.bot: commit-queue-
Alexander Pavlov (apavlov)
Comment 1 2012-11-30 09:29:35 PST
Andrey Kosyakov
Comment 2 2012-11-30 15:52:06 PST
Comment on attachment 176980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176980&action=review > Source/WebCore/page/FrameView.cpp:1339 > + if (InspectorInstrumentation::shouldEmulatePrintMedia(m_frame.get())) > + overrideType = "print"; Why not make it generic -- i.e. allow to override to arbitrary media type?
Pavel Feldman
Comment 3 2012-11-30 17:36:09 PST
Comment on attachment 176980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176980&action=review > Source/WebCore/inspector/Inspector.json:435 > + { "name": "enabled", "type": "boolean", "description": "Whether the printed media emulation should be enabled." } Lets keep it generic in the protocol. > Source/WebCore/inspector/front-end/SettingsScreen.js:331 > + const p = document.createElement("p"); This should be a part of the overrides >> Source/WebCore/page/FrameView.cpp:1339 >> + overrideType = "print"; > > Why not make it generic -- i.e. allow to override to arbitrary media type? +1
Alexander Pavlov (apavlov)
Comment 4 2012-12-03 01:04:21 PST
Build Bot
Comment 5 2012-12-03 01:22:38 PST
Alexander Pavlov (apavlov)
Comment 6 2012-12-03 01:57:57 PST
Pavel Feldman
Comment 7 2012-12-03 10:58:02 PST
Comment on attachment 177211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177211&action=review > Source/WebCore/inspector/InspectorPageAgent.cpp:378 > + accidental change?
Andrey Kosyakov
Comment 8 2012-12-03 11:08:14 PST
Comment on attachment 177211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177211&action=review > Source/WebCore/inspector/front-end/OverridesView.js:615 > + if (mediaType === "all") { > + // "all" is not a device-specific media type. > + continue; > + } Just remove "all" from the array? > Source/WebCore/inspector/front-end/OverridesView.js:621 > + if (mediaType === "print") > + mediaSelectElement.selectedIndex = i; this looks a bit fishy... wouldn't it be better to persist the default selection?
Alexander Pavlov (apavlov)
Comment 9 2012-12-04 01:54:46 PST
Comment on attachment 177211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177211&action=review >> Source/WebCore/inspector/InspectorPageAgent.cpp:378 >> + > > accidental change? Indeed! >> Source/WebCore/inspector/front-end/OverridesView.js:615 >> + } > > Just remove "all" from the array? I had thought about this at first, but having a "pure-device" set of media types next to the full set thereof sounds bad (in fact, we do need the latter in the CSS tokenizer, so this could be refactored barring one weird value there). >> Source/WebCore/inspector/front-end/OverridesView.js:621 >> + mediaSelectElement.selectedIndex = i; > > this looks a bit fishy... wouldn't it be better to persist the default selection? A nice idea. Implemented.
Alexander Pavlov (apavlov)
Comment 10 2012-12-04 01:59:24 PST
Created attachment 177450 [details] Patch for landing
WebKit Review Bot
Comment 11 2012-12-04 02:18:22 PST
Comment on attachment 177450 [details] Patch for landing Rejecting attachment 177450 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit eb9e8cc..96514be HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at 96514be36664e1430392d9b2fd3200c2aab861f9 but expected eb9e8ccd030f66f658b9fcdabeeb134aaaf82783 ! eb9e8cc..96514be master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15120776
Alexander Pavlov (apavlov)
Comment 12 2012-12-04 02:26:43 PST
Alexander Pavlov (apavlov)
Comment 13 2013-03-25 03:28:18 PDT
*** Bug 13530 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.