Bug 183708

Summary: Web Inspector: Elements: "Force Print Media Styles" should not persist across Web Inspector sessions
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: 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 Flags
Patch
none
[Animated GIF] With patch applied none

Description Nikita Vasilyev 2018-03-16 14:33:51 PDT
Several people accidentally enabled print stylesheets by clicking the printer icon in the Elements tab
and didn't know what happened and how to change it back.

Don't save "Force Print Media Styles" state to WI localStorage,
so re-opening Web Inspector should always have "Force Print Media Styles" disabled.
Comment 1 Nikita Vasilyev 2018-03-16 14:34:03 PDT
<rdar://problem/36452183>
Comment 2 Nikita Vasilyev 2018-03-16 14:40:23 PDT
Created attachment 335971 [details]
Patch
Comment 3 Nikita Vasilyev 2018-03-16 14:47:04 PDT
Created attachment 335973 [details]
[Animated GIF] With patch applied
Comment 4 Matt Baker 2018-03-16 14:59:29 PDT
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 5 Nikita Vasilyev 2018-03-16 15:09:13 PDT
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 6 Matt Baker 2018-03-16 15:16:36 PDT
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.
Comment 7 Nikita Vasilyev 2018-03-16 15:18:48 PDT
Whoops, I misread it. I still prefer to have a state variable in Main.js.
Comment 8 WebKit Commit Bot 2018-03-16 15:33:51 PDT
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 9 Nikita Vasilyev 2018-03-16 16:19:04 PDT
Comment on attachment 335971 [details]
Patch

Unrelated test failure.
Comment 10 WebKit Commit Bot 2018-03-16 16:43:46 PDT
Comment on attachment 335971 [details]
Patch

Clearing flags on attachment: 335971

Committed r229686: <https://trac.webkit.org/changeset/229686>
Comment 11 WebKit Commit Bot 2018-03-16 16:43:47 PDT
All reviewed patches have been landed.  Closing bug.