[webkit-changes] [WebKit/WebKit] 0b8752: Web Inspector: more then one popover can be displa...

Devin Rousso noreply at github.com
Wed May 17 10:25:20 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 0b875224e976587c77b3a9a9e8d0cd4992f94376
      https://github.com/WebKit/WebKit/commit/0b875224e976587c77b3a9a9e8d0cd4992f94376
  Author: Devin Rousso <hi at devinrousso.com>
  Date:   2023-05-17 (Wed, 17 May 2023)

  Changed paths:
    M Source/WebInspectorUI/UserInterface/Views/Popover.js

  Log Message:
  -----------
  Web Inspector: more then one popover can be displayed in the same time
https://bugs.webkit.org/show_bug.cgi?id=253557
<rdar://problem/106741263>

Reviewed by Patrick Angle.

Because all `WI.Popover` shared the same `<canvas>` for drawing the background, a second `WI.Popover` being shown would change the background of the first `WI.Popover`.

This fix has two parts.

1) Programmatically enforce that there can only be one `WI.Popver` "visible" at a time. Note that "visible" is defined by `WI.Popover.prototype.get visible` (i.e. attached to the DOM *and* not actively being dismissed). If a second `WI.Popover` is shown, `dismiss` the first `WI.Popover`. This prevents the two `WI.Popover` from overlapping (though there is a brief period where both can be visible while the first `WI.Popover` transitions to full transparency, but this is a better UX).

2) Instead of having a single globally shared `<canvas>`, have a global pool of `<canvas>` that are shared by all `WI.Popover`. When a `WI.Popover` is shown, it removes any `<canvas>` from the pool and uses it until it's `dismiss`. Note that if a second `WI.Popover` is shown during this time, another `<canvas>` will be created as the first `WI.Popover` would have removed the only `<canvas>` in the global pool of `<canvas>`. In order to avoid keeping that second `<canvas>` alive forever (since it would sit in the global pool of `<canvas>` until a second `WI.Popover` used it), each `<canvas>` in the global pool of `<canvas>` is wrapped in a `WeakRef`, so the worst that could happen is an empty `WeakRef` being kept alive.

With both of these, we now have a more ideal experience where
- only one `WI.Popover` will be "visible" at a time, such that a second `WI.Popover` being shown will `dismiss` the first `WI.Popover`
- if two `WI.Popover` are actually visible at the same time, each will have their own `<canvas>`, ensuring neither clobbers the background of the other

* Source/WebInspectorUI/UserInterface/Views/Popover.js:
(WI.Popover):
(WI.Popover.prototype.dismiss):
(WI.Popover.prototype.handleEvent):
(WI.Popover.prototype._drawBackground):
(WI.Popover.prototype._drawFrame):
(WI.Popover._getCanvasContext): Deleted.

Canonical link: https://commits.webkit.org/264159@main




More information about the webkit-changes mailing list