Bug 188891 – REGRESSION (r232991): Switching to dark mode in Mail does not update the message view to be transparent

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Timothy Hatcher]

<rdar://problem/42344352>

[Radar WebKit Bug Importer]

<rdar://problem/43648659>

[Timothy Hatcher]

Created attachment 347934 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 347934 [details]
Patch

Needs a test.

[Timothy Hatcher]

Comment on attachment 347934 [details]
Patch

Yesterday and this morning I tried to write an API test for this. I was not able to recreate the bug in the test harness -- only in Mail. 

In the test runner, rootBackgroundColorOrTransparencyChanged() ends upon getting called with extendedBackgroundColorChanged as true and then false in short succession. So the bad early return was blocking updateRootLayerConfiguration() first, but then ends up getting it scheduled soon after then the color does not change.

Maybe you have a suggestion on how to write a test for this, but I'm at a loss.

Can we just get this landed? Reverting the bad part of r232991 and returning us to the old behavior of no early return.

[Timothy Hatcher]

Typo fix: "...scheduled soon after when the color does not change."

[Simon Fraser (smfr)]

You should be able to change view transparency in a layout test, then dump the graphics layer tree to show the "contentsOpaque" on the page tile layers (assuming we can dump them. if not, make a way to dump them!).

[Timothy Hatcher]

(In reply to Simon Fraser (smfr) from comment #6)
> You should be able to change view transparency in a layout test, then dump
> the graphics layer tree to show the "contentsOpaque" on the page tile layers
> (assuming we can dump them. if not, make a way to dump them!).

That is what I was doing, but with an API test. And it wasn't triggering the problem observed in Mail.

[Timothy Hatcher]

I get the same results in a layout test as I did with the API test -- I can't reproduce  the bug. The contentsOpaque flag always get cleared.

Before:
(GraphicsLayer
  (anchor 0.00 0.00)
  (bounds 785.00 1024.00)
  (children 1
    (GraphicsLayer
      (bounds 785.00 1024.00)
      (contentsOpaque 1)
      (tile cache coverage 0, 0 785 x 1024)
      (tile size 785 x 512)
      (top left tile 0, 0 tiles grid 1 x 2)
      (in window 1)
    )
  )
)

After setDrawsBackground(false):
(GraphicsLayer
  (anchor 0.00 0.00)
  (bounds 785.00 1024.00)
  (children 1
    (GraphicsLayer
      (bounds 785.00 1024.00)
      (tile cache coverage 0, 0 785 x 1024)
      (tile size 785 x 512)
      (top left tile 0, 0 tiles grid 1 x 2)
      (in window 1)
    )
  )
)

[Timothy Hatcher]

My test used an internals.setDrawsBackground(false) that triggers the same code path Mail is using.

[WebKit Commit Bot]

Comment on attachment 347934 [details]
Patch

Clearing flags on attachment: 347934

Committed r239414: <https://trac.webkit.org/changeset/239414>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.