Improved the reporting of background color information.

Improved the reporting of background color information.

Previously only the background color in the element's style was exposed. More often than not, this was set to transparent.
With this change, the background colors of all the layout objects from the current object up to the document object are blended until we reach an opacity of less than 50%.
R=dmazzoni@chromium.org, aboxhall@chromium.org
BUG=595880
TESTED=layout test

Committed: https://crrev.com/36a0acdb9f8022148a26ab6198098a44964c19fd
Cr-Commit-Position: refs/heads/master@{#396242}

[dmazzoni, 2016-05-23 23:03:04]

Why an opacity of 50%? Would it work to just go until 100% opacity?

Alice should be the primary reviewer here, she should have a better idea.

[aboxhall, 2016-05-24 17:06:34]

https://codereview.chromium.org/2006043002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2006043002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:843: for
(LayoutObject* layoutObject = getLayoutObject(); layoutObject;
Unfortunately, walking up the parent chain often isn't sufficient because of
absolute positioning.

I wrote code for this in devtools:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

I wonder if we can reuse that here, somehow?

https://codereview.chromium.org/2006043002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:852: if
(blendedColor.alpha() > 128)
I share Dominic's concern here: for example, 51% red over blue appears purple,
not blue.

[aboxhall1, 2016-05-24 19:01:12]

aboxhall@google.com changed reviewers:
+ aboxhall@google.com

[aboxhall1, 2016-05-24 19:01:13]

Here are the tests I wrote for the devtools code:
https://chromium.googlesource.com/chromium/src/+/989a790441d7bd00365a7de1251d...

[blink-reviews, 2016-05-25 19:34:57]

As discussed offline, I changed the stopping condition to opacity 100% 
and I cached the background color for performance, as I proposed.
Please have another look.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-05-25 19:34:57]

As discussed offline, I changed the stopping condition to opacity 100% 
and I cached the background color for performance, as I proposed.
Please have another look.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[aboxhall, 2016-05-25 19:36:13]

On 2016/05/25 19:34:57, chromium-reviews wrote:
> As discussed offline, I changed the stopping condition to opacity 100% 
> and I cached the background color for performance, as I proposed.
> Please have another look.
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

Needs reupload?

[blink-reviews, 2016-05-25 20:24:06]

Sorry, the upload script failed. Now it should be uploaded.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-05-25 20:24:06]

Sorry, the upload script failed. Now it should be uploaded.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[aboxhall, 2016-05-25 20:59:07]

LGTM with nits.

https://codereview.chromium.org/2006043002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/background-color.html (right):

https://codereview.chromium.org/2006043002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/background-color.html:41: },
'Ensures that the color of the parent is exposed if the text is transparent.');
#transparent doesn't seem to me to have transparent text, but rather a
transparent background.

https://codereview.chromium.org/2006043002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/background-color.html:53:
assert_equals(axBlended.backgroundColor, 0xffbfbfbf);  // Lite red
Comment is inaccurate: that's more of a light grey (which makes sense given
you're blending white and grey).

https://codereview.chromium.org/2006043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2006043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:842: //
Color::blend should be called like this: background.blend(foreground).
Great comment :)

[blink-reviews, 2016-05-25 21:17:09]

Comments addressed.
I'll submit now.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-05-25 21:17:09]

Comments addressed.
I'll submit now.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2016-05-25 21:17:16]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-25 21:17:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
aboxhall@chromium.org
Link to the patchset: https://codereview.chromium.org/2006043002/#ps40001
(title: "Addressed nits.")

[commit-bot: I haz the power, 2016-05-25 21:17:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006043002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006043002/40001

[commit-bot: I haz the power, 2016-05-25 21:27:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-25 21:27:42]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dmazzoni, 2016-05-25 21:33:05]

lgtm

[nektarios, 2016-05-25 21:34:20]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-05-25 21:35:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006043002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006043002/40001

[commit-bot: I haz the power, 2016-05-25 21:49:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-25 21:49:04]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[nektarios, 2016-05-25 22:20:12]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-25 22:20:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, aboxhall@chromium.org
Link to the patchset: https://codereview.chromium.org/2006043002/#ps60001
(title: "Fixed browser tests.")

[commit-bot: I haz the power, 2016-05-25 22:21:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006043002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006043002/60001

[commit-bot: I haz the power, 2016-05-26 00:06:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-26 00:06:23]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-05-26 15:16:20]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-26 15:16:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, aboxhall@chromium.org
Link to the patchset: https://codereview.chromium.org/2006043002/#ps80001
(title: "Fixed another layout test.")

[commit-bot: I haz the power, 2016-05-26 15:16:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006043002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006043002/80001

[commit-bot: I haz the power, 2016-05-26 16:45:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-26 16:45:07]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-05-26 16:58:54]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-05-26 16:58:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, aboxhall@chromium.org
Link to the patchset: https://codereview.chromium.org/2006043002/#ps100001
(title: "Added blank line to test expectations.")

[commit-bot: I haz the power, 2016-05-26 16:59:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006043002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006043002/100001

[commit-bot: I haz the power, 2016-05-26 18:47:29]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-26 18:48:54]

Description was changed from

==========
Improved the reporting of background color information.

Previously only the background color in the element's style was exposed. More
often than not, this was set to transparent.
With this change, the background colors of all the layout objects from the
current object up to the document object are blended until we reach an opacity
of less than 50%.
R=dmazzoni@chromium.org, aboxhall@chromium.org
BUG=595880
TESTED=layout test
==========

to

==========
Improved the reporting of background color information.

Previously only the background color in the element's style was exposed. More
often than not, this was set to transparent.
With this change, the background colors of all the layout objects from the
current object up to the document object are blended until we reach an opacity
of less than 50%.
R=dmazzoni@chromium.org, aboxhall@chromium.org
BUG=595880
TESTED=layout test

Committed: https://crrev.com/36a0acdb9f8022148a26ab6198098a44964c19fd
Cr-Commit-Position: refs/heads/master@{#396242}
==========

[commit-bot: I haz the power, 2016-05-26 18:48:55]

Patchset 6 (id:??) landed as
https://crrev.com/36a0acdb9f8022148a26ab6198098a44964c19fd
Cr-Commit-Position: refs/heads/master@{#396242}