Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(150)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 442563002: Fix getComputedStyle() for area element (Closed)

Created:
6 years, 4 months ago by zhaoze.zhou
Modified:
6 years, 4 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com, rwlbuis, rune+blink
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Fix getComputedStyle() for area element The getComputedStyle() function didn't work as we expected for the area. The reason is chrome set display area as none, this patch fix this problem. This behavior matches Firefox. The Internet Explorer 11 set area display as none. BUG=392759 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180610

Patch Set 1 : #

Total comments: 1

Patch Set 2 : Add test file #

Patch Set 3 : Fix test case #

Total comments: 8

Patch Set 4 : Fix virtual test case #

Total comments: 24

Patch Set 5 : Fix code style #

Total comments: 9

Patch Set 6 : Fix style #

Total comments: 3

Patch Set 7 : Remove tags #

Total comments: 2

Patch Set 8 : Remove tags #

Patch Set 9 : fix mac blink test #

Patch Set 10 : Try to use rebaseline #

Patch Set 11 : Try to use rebaseline #

Unified diffs Side-by-side diffs Delta from patch set Stats (+171 lines, -14 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 1 chunk +28 lines, -0 lines 0 comments Download
M LayoutTests/accessibility/image-map1.html View 1 2 3 4 5 1 chunk +6 lines, -6 lines 0 comments Download
M LayoutTests/accessibility/image-map1-expected.txt View 1 2 1 chunk +6 lines, -6 lines 0 comments Download
M LayoutTests/accessibility/image-map2-expected.txt View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/area-computedStyle.html View 1 2 3 4 5 6 7 1 chunk +30 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css/area-computedStyle-expected.txt View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-case-expected.txt View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-circle-focus-ring-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-outline-color-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-zero-outline-width-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-focus-ring-zoom-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-overflowing-circle-focus-ring-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-overflowing-polygon-focus-ring-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/images/imagemap-polygon-focus-ring-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/http/tests/misc/acid3-expected.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/tables/mozilla/bugs/bug1296-expected.txt View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/tables/mozilla/bugs/bug1430-expected.txt View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-case-expected.txt View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-circle-focus-ring-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-zero-outline-width-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-focus-ring-zoom-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-overflowing-circle-focus-ring-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-overflowing-polygon-focus-ring-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/linux/virtual/deferred/fast/images/imagemap-polygon-focus-ring-expected.txt View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/fast/images/imagemap-case-expected.txt View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-circle-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/fast/images/imagemap-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-focus-ring-outline-color-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/fast/images/imagemap-focus-ring-zero-outline-width-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-focus-ring-zoom-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-overflowing-circle-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-overflowing-polygon-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/fast/images/imagemap-polygon-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/http/tests/misc/acid3-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/tables/mozilla/bugs/bug1296-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -0 lines 0 comments Download
A + LayoutTests/platform/mac/tables/mozilla/bugs/bug1430-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -0 lines 0 comments Download
A + LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-case-expected.txt View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-circle-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-zero-outline-width-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-focus-ring-zoom-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-overflowing-circle-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-overflowing-polygon-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/mac/virtual/deferred/fast/images/imagemap-polygon-focus-ring-expected.txt View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/html.css View 1 2 3 4 5 6 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 22 (0 generated)
rwlbuis
Needs a test :) Also the commit log will need a description. https://codereview.chromium.org/442563002/diff/20001/Source/core/css/html.css File Source/core/css/html.css ...
6 years, 4 months ago (2014-08-05 19:36:23 UTC) #1
Inactive
https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode1 LayoutTests/fast/css/area-computedStyle.html:1: <html> missing DOCTYPE. https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode6 LayoutTests/fast/css/area-computedStyle.html:6: function runtest(){ missing space ...
6 years, 4 months ago (2014-08-06 21:40:44 UTC) #2
maheshkk
Some nits about testcase style. https://codereview.chromium.org/442563002/diff/60001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/60001/LayoutTests/fast/css/area-computedStyle.html#newcode1 LayoutTests/fast/css/area-computedStyle.html:1: <html> Leave a comment ...
6 years, 4 months ago (2014-08-06 21:44:11 UTC) #3
Inactive
https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode5 LayoutTests/fast/css/area-computedStyle.html:5: <script> Missing a description("..."); https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode15 LayoutTests/fast/css/area-computedStyle.html:15: var expectedColor = ...
6 years, 4 months ago (2014-08-06 21:44:38 UTC) #4
Inactive
https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode14 LayoutTests/fast/css/area-computedStyle.html:14: var s = window.getComputedStyle(area,null); On 2014/08/06 21:40:43, Chris Dumez ...
6 years, 4 months ago (2014-08-06 21:47:02 UTC) #5
Inactive
https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/80001/LayoutTests/fast/css/area-computedStyle.html#newcode10 LayoutTests/fast/css/area-computedStyle.html:10: eventSender.mouseMoveTo(75, 75); This looks unsafe, you would probably get ...
6 years, 4 months ago (2014-08-06 21:50:53 UTC) #6
zhaoze.zhou
https://codereview.chromium.org/442563002/diff/60001/Source/core/css/html.css File Source/core/css/html.css (right): https://codereview.chromium.org/442563002/diff/60001/Source/core/css/html.css#newcode1014 Source/core/css/html.css:1014: a:-webkit-any-link { On 2014/08/06 21:44:11, maheshkk wrote: > are ...
6 years, 4 months ago (2014-08-07 15:09:20 UTC) #7
Inactive
https://codereview.chromium.org/442563002/diff/220001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/220001/LayoutTests/fast/css/area-computedStyle.html#newcode10 LayoutTests/fast/css/area-computedStyle.html:10: <body onload="runtest()"> runTest() https://codereview.chromium.org/442563002/diff/220001/LayoutTests/fast/css/area-computedStyle.html#newcode15 LayoutTests/fast/css/area-computedStyle.html:15: <script src="../../resources/js-test.js"></script> Since you ...
6 years, 4 months ago (2014-08-07 15:34:23 UTC) #8
Inactive
https://codereview.chromium.org/442563002/diff/220001/LayoutTests/accessibility/image-map1.html File LayoutTests/accessibility/image-map1.html (left): https://codereview.chromium.org/442563002/diff/220001/LayoutTests/accessibility/image-map1.html#oldcode10 LayoutTests/accessibility/image-map1.html:10: Unrelated change. https://codereview.chromium.org/442563002/diff/220001/LayoutTests/accessibility/image-map1.html File LayoutTests/accessibility/image-map1.html (right): https://codereview.chromium.org/442563002/diff/220001/LayoutTests/accessibility/image-map1.html#newcode23 LayoutTests/accessibility/image-map1.html:23: ditto.
6 years, 4 months ago (2014-08-07 15:36:07 UTC) #9
zhaoze.zhou
yosin@chromium.org. Please take a look at Source/core/css/html.css file. This can fix the bug you gave. ...
6 years, 4 months ago (2014-08-07 16:48:30 UTC) #10
yosin_UTC9
On 2014/08/07 16:48:30, zhaoze.zhou wrote: > mailto:yosin@chromium.org. Please take a look at > Source/core/css/html.css file. ...
6 years, 4 months ago (2014-08-08 01:36:39 UTC) #11
esprehn
lgtm, this is reasonable, the other thing we could do is use nonRendererStyle(). I suspect ...
6 years, 4 months ago (2014-08-19 01:41:28 UTC) #12
Inactive
https://codereview.chromium.org/442563002/diff/280001/LayoutTests/fast/css/area-computedStyle.html File LayoutTests/fast/css/area-computedStyle.html (right): https://codereview.chromium.org/442563002/diff/280001/LayoutTests/fast/css/area-computedStyle.html#newcode2 LayoutTests/fast/css/area-computedStyle.html:2: <map name="imagemap1"> nit: Maybe we could start with the ...
6 years, 4 months ago (2014-08-19 17:46:11 UTC) #13
zhaoze.zhou
The CQ bit was checked by zhaoze.zhou@partner.samsung.com
6 years, 4 months ago (2014-08-19 17:58:34 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zhaoze.zhou@partner.samsung.com/442563002/320001
6 years, 4 months ago (2014-08-19 17:59:13 UTC) #15
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: mac_blink_rel on tryserver.blink ...
6 years, 4 months ago (2014-08-19 19:00:48 UTC) #16
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-19 19:02:26 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/20435)
6 years, 4 months ago (2014-08-19 19:02:27 UTC) #18
zhaoze.zhou
The CQ bit was checked by zhaoze.zhou@partner.samsung.com
6 years, 4 months ago (2014-08-19 23:23:37 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zhaoze.zhou@partner.samsung.com/442563002/380001
6 years, 4 months ago (2014-08-19 23:23:55 UTC) #20
commit-bot: I haz the power
Committed patchset #11 (380001) as 180610
6 years, 4 months ago (2014-08-20 00:23:25 UTC) #21
tasak
5 years, 10 months ago (2015-02-16 06:12:05 UTC) #22
Message was sent while issue was closed. 
A revert of this CL (patchset #11 id:380001) has been created in
https://codereview.chromium.org/930763002/ by tasak@google.com.

The reason for reverting is: This causes crbug.com/455253.
.

Powered by Google App Engine
This is Rietveld 408576698