Summary: | Web Inspector: Elements: "Force Print Media Styles" should not persist across Web Inspector sessions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||
Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, inspector-bugzilla-changes, mattbaker, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Nikita Vasilyev
2018-03-16 14:33:51 PDT
Created attachment 335971 [details]
Patch
Created attachment 335973 [details]
[Animated GIF] With patch applied
Comment on attachment 335971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335971&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Main.js:182 > + this.printStylesEnabled = false; This could be a static property of DOMTreeContentView, since it's now the only place it's used. But I'll leave it up to you. Comment on attachment 335971 [details] Patch (In reply to Matt Baker from comment #4) > Comment on attachment 335971 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335971&action=review > > r=me > > > Source/WebInspectorUI/UserInterface/Base/Main.js:182 > > + this.printStylesEnabled = false; > > This could be a static property of DOMTreeContentView, since it's now the > only place it's used. But I'll leave it up to you. Consider the following case: 1. Open a wikipedia article, say https://en.wikipedia.org/wiki/Metropolitan_statistical_area 2. Click "Force Print Media Styles" 3. Navigate to a different article, such as https://en.wikipedia.org/wiki/U.S._state Expected: Print styles stay enabled. If I make printStylesEnabled a private property of DOMTreeContentView, print styles would reset back to disabled. Comment on attachment 335971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335971&action=review >>> Source/WebInspectorUI/UserInterface/Base/Main.js:182 >>> + this.printStylesEnabled = false; >> >> This could be a static property of DOMTreeContentView, since it's now the only place it's used. But I'll leave it up to you. > > Consider the following case: > 1. Open a wikipedia article, say https://en.wikipedia.org/wiki/Metropolitan_statistical_area > 2. Click "Force Print Media Styles" > 3. Navigate to a different article, such as https://en.wikipedia.org/wiki/U.S._state > > Expected: > Print styles stay enabled. > > If I make printStylesEnabled a private property of DOMTreeContentView, print styles would reset back to disabled. I suggested making it a static (class) property. Using a private property would also cause print styles to be reset when removing and adding the Element tab. It's fine as-is in Main.js. Whoops, I misread it. I still prefer to have a state variable in Main.js. Comment on attachment 335971 [details] Patch Rejecting attachment 335971 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 335971, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: org/git/WebKit 8fd1f9b8800..3238fbbc75c master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 229683 = 8fd1f9b880041cdae470c6c642bce201f506e686 r229684 = 3238fbbc75c4392730fb9fc646e5f497d8c9b2b7 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/6987624 Comment on attachment 335971 [details]
Patch
Unrelated test failure.
Comment on attachment 335971 [details] Patch Clearing flags on attachment: 335971 Committed r229686: <https://trac.webkit.org/changeset/229686> All reviewed patches have been landed. Closing bug. |