1412150 - Google Docs text sometimes appears too light

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Mark Côté [:mcote]]

Attachment: View of Google Doc with some text appearing correctly and some not

I'm using the latest Nightly from 2017/10/26.  I do not recall ever seeing this before I updated earlier today.  The text on many Google Docs appears, in places, too light (see screenshot).  It seems to only occur when there are comments (and maybe only after a certain number of them).  The problem seems to come and go as I scroll or hover over comments, although when it manifests it seems to be present more often than not.  It makes reading those docs very difficult and annoying.

Comment 1

[alex_mayorga]

¡Hola Mike!

A webcompat thingie perhaps?

¡Gracias!
Alex

Comment 2

[Mark Côté [:mcote]]

FYI this is still occurring with this morning's Nightly.  I was not able to reproduce it on 56.0.2.

Comment 3

[Mark Côté [:mcote]]

Putting the doc into view mode appears to fix it.  Switching back to suggestion mode reveals the bug again.  So it really seems related to comments in the doc.

Comment 4

[Mike Taylor [:miketaylr]]

Mark, could you share that doc with [e-mail] (if it's something I'm allowed to look at) so I can take a look?

Comment 5

[Mark Côté [:mcote]]

Done.  It's a public doc anyway.

Comment 6

[Mark Côté [:mcote]]

For anyone else wanting to repro: https://docs.google.com/document/d/1QPZG2YObvIsEf8fddJ-DngV2B8C2kL0jjvkXmxBwdfU/edit

Comment 7

[Mike Taylor [:miketaylr]]

This might be graphics related, I can't reproduce on my machine (win10 or macOS). Also note you need to be logged in with LDAP to see the comments (which are part of STR).

Comment 8

[Mark Côté [:mcote]]

I have now reproduced it on two Windows 10 machines.  One is my work XPS13 laptop.  The other is a desktop with a Radeon RX 460.  It took a bit more clicking & scrolling to reproduce it on the latter, but not too long (10-15 seconds maybe).

Comment 9

[Alice0775 White]

[Tracking Requested - why for this release]:

I can also reproduce the problem n Windows10 Nightly58.0a1 64bit.

Reproducible: 100%

Steps To Reproduce:
1. Launch Nightly
2. Open https://docs.google.com/document/d/1QPZG2YObvIsEf8fddJ-DngV2B8C2kL0jjvkXmxBwdfU/edit in new tab[A]
3. After complete loading the page, Scroll to the bottom
4. Click [+] button in tabstrip to open a new tab
5. Switch back to the previous tab[A]

Actual Results:
After for a seconds, text becomes too light.


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4beb0994d438f9dc4f82b44648997913aa5d6355&tochange=b97eefb07e39f92a6bc21be23cb7bb566fbac5cd

Regressed by: bug 1409871

@:rhunt,
your punch of patch seems to cause the regression, Can you look into this?

Comment 11

[Ryan Hunt [:rhunt] (on leave until early May)]

Yes, I'll look at this straight away.

Comment 12

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: content-client-component-alpha.patch

Wrong bug number on the commit comment, will fix that later.

I believe this is also the cause behind bug 1412367, as I was no longer able to reproduce that after this fix.

Comment 13

[Marcia Knous [:marcia]]

Tracking 58+ for this reproducible issue.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/805cb22ff077
Be sure to invalidate the buffer when content changes and we cannot reuse the buffer. (bug 1412150, r=nical)

Comment 15

[Ryan Hunt [:rhunt] (on leave until early May)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=57dc29e677ee1c137c0c315b9034602ab9fc3bc2

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Can we please try to land a testcase that would have caught this issue?

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/805cb22ff077

Comment 19

[Ryan Hunt [:rhunt] (on leave until early May)]

Yes, I will look into adding a test for this.

Comment 21

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/805cb22ff077

Comment 24

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: ca-enter-displayport-change.patch

This is a testcase based off of component-alpha-exit-1.html that triggers the regression before the fix.

The test consists of two scrollable divs, with the second one having some text that requires component alpha, but scrolled out of view. The test waits for a paint, then removes the first scrollable div. This causes the second scrollable div to be layerized and given a display port. The display port is big enough to include the text causing the layer to transition to component alpha.

This causes a frame size change, a transition to component alpha, and an overlapping draw region, all of which are necessary to trigger this bug.

Comment 25

[Markus Stange [:mstange]]

Comment on attachment 8925382 [details] [diff] [review]
ca-enter-displayport-change.patch

Review of attachment 8925382 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/layers/component-alpha-enter-1.html
@@ +8,5 @@
> +
> +    window.addEventListener("MozAfterPaint", function() {
> +      // Remove dummy to make scrollbox the first scroll element giving it a
> +      // display port. The display port will then contain the text forcing the
> +      // layer to transition to component alpha.

Oh, and FrameLayerBuilder will recycle the PaintedLayer from the old scroll frame contents and use it for the contents of the newly-active scroll frame. Maybe that should be added to the comment.

Comment 26

[Ryan Hunt [:rhunt] (on leave until early May)]

Oh I used the wrong bug number. The reftest was pushed with bug 1409871, which is the bug that introduced this regression. See bug 1409871 comment 90.

Comment 27

[Ryan Hunt [:rhunt] (on leave until early May)]

Per bug 1409871 comment 91, this has a test now.