Bug 192963

Summary: REGRESSION(r?): Web Inspector: Popovers have inset shadows
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, hi, inspector-bugzilla-changes, joepeck, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] Bug
none
Patch
none
[Image] Before Patch is applied
none
[Image] After Patch is applied none

Description Nikita Vasilyev 2018-12-20 17:22:04 PST
Created attachment 357905 [details]
[Image] Bug

This doesn't seem intentional.
Comment 1 Radar WebKit Bug Importer 2018-12-20 17:22:22 PST
<rdar://problem/46888679>
Comment 2 Devin Rousso 2019-02-21 16:23:05 PST
Created attachment 362664 [details]
Patch
Comment 3 Devin Rousso 2019-02-21 16:23:42 PST
Created attachment 362665 [details]
[Image] Before Patch is applied
Comment 4 Devin Rousso 2019-02-21 16:24:07 PST
Created attachment 362666 [details]
[Image] After Patch is applied
Comment 5 Nikita Vasilyev 2019-02-21 16:41:41 PST
Do you know why this broke?
Comment 6 Joseph Pecoraro 2019-02-21 16:44:36 PST
Comment on attachment 362664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362664&action=review

> Source/WebInspectorUI/UserInterface/Views/Popover.css:36
>      --popover-shadow-color: hsla(0, 0%, 0%, 0.5);

Can we reduce the padding now that the popover feels a little larger than it was before? Each of the `padding` declarations below could drop a pixel or two.
Comment 7 Joseph Pecoraro 2019-02-21 16:47:07 PST
Comment on attachment 362664 [details]
Patch

The  macOS system blue seems much larger. For an example of a popover right click the URL bar and select "Settings for this website...". That shadow seems much larger and clearer than this one.
Comment 8 Joseph Pecoraro 2019-02-21 16:47:32 PST
> The  macOS system blue

system *popover blur*
Comment 9 Matt Baker 2019-02-21 17:03:56 PST
I’d really like to know what change caused the regression in the first place. I feel like this happened relatively recently.
Comment 10 Devin Rousso 2019-02-21 17:04:48 PST
(In reply to Nikita Vasilyev from comment #5)
> Do you know why this broke?
(In reply to Matt Baker from comment #9)
> I’d really like to know what change caused the regression in the first place. I feel like this happened relatively recently.
From what I could find, this code has existed since before Web Inspector was added to open source.
Comment 11 Devin Rousso 2019-02-21 17:07:56 PST
Comment on attachment 362664 [details]
Patch

(In reply to Joseph Pecoraro from comment #7)
> The  macOS system popover blur seems much larger. For an example of a popover right click the URL bar and select "Settings for this website...". That shadow seems much larger and clearer than this one.

View in context: https://bugs.webkit.org/attachment.cgi?id=362664&action=review

>> Source/WebInspectorUI/UserInterface/Views/Popover.css:36
>>      --popover-shadow-color: hsla(0, 0%, 0%, 0.5);
> 
> Can we reduce the padding now that the popover feels a little larger than it was before? Each of the `padding` declarations below could drop a pixel or two.

If we want to make the popover match (or be closer to) the system blur, we'd actually need to increase the padding, as otherwise the blur will get cut off by the bounds of the <canvas> itself.
Comment 12 BJ Burg 2019-02-22 14:51:34 PST
Comment on attachment 362664 [details]
Patch

r=me
Comment 13 WebKit Commit Bot 2019-02-22 20:10:06 PST
Comment on attachment 362664 [details]
Patch

Clearing flags on attachment: 362664

Committed r241981: <https://trac.webkit.org/changeset/241981>
Comment 14 WebKit Commit Bot 2019-02-22 20:10:08 PST
All reviewed patches have been landed.  Closing bug.