New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 697337 link

Starred by 5 users

Issue metadata

Status: Fixed
Merged: issue 685174
Owner:
NOT IN USE
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression

Blocking:
issue 704576



Sign in to add a comment

Styles on anchor tags don't get applied when the focus state has outline none AND ::selection has some styling defined

Reported by aruna....@gmail.com, Mar 1 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3026.0 Safari/537.36

Steps to reproduce the problem:
1 define styles for a:focus { }
2 define styles for ::selection { }
3 toggle the color of the anchor tag when clicked

(Minimal representation of the issue: http://codepen.io/anon/pen/OpyGdy)

What is the expected behavior?
The color of the anchor tag should toggle when clicked

What went wrong?
The color of the anchor tag is not changing even though the class is getting applied

Did this work before? Yes 57.0.2987

Does this work in other browsers? Yes

Chrome version: 58.0.3026.0  Channel: canary
OS Version: OS X 10.12.1
Flash Version: Shockwave Flash 25.0 r0

This is observed only in Canary

 

Comment 1 by woxxom@gmail.com, Mar 1 2017

Bisect: 451988 (good) - 451997 (bad), 58.0.3021.0
https://chromium.googlesource.com/chromium/src/+log/a88a17ed..fa5fec25?pretty=fuller
Suspecting r451992 ( issue 685174 ) "Repaint selection when element with ::selection style is recalculated"

Comment 2 by woxxom@gmail.com, Mar 1 2017

The issue is known it seems according to  https://crbug.com/685174#c13 

>You're right. That fix was not good enough.
Labels: PaintTeamTriaged-20170301 BugSource-User
Mergedinto: 685174
Status: Duplicate (was: Unconfirmed)
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c63e571885b21aa510dc323ecb1c5bc18f1ed29b

commit c63e571885b21aa510dc323ecb1c5bc18f1ed29b
Author: rune <rune@opera.com>
Date: Fri Mar 03 11:12:11 2017

needsPaintInvalidation() should not return true for selection.

In [1] we introduced a paint invalidation constant for repainting
selection. needsPaintInvalidation() started to also return true when
only the selection needed paint invalidation. Callers of that method
assumes that true will cause at least a full element repaint which made
us skip setting the diff to invalidate the object and only invalidate
the selection in some cases. Instead, only return true for
PaintInvalidationObject and PaintInvalidationSubtree.

[1] https://crrev.com/eff357ef

R=mstensho@opera.com
BUG= 697337 

Review-Url: https://codereview.chromium.org/2727843004
Cr-Commit-Position: refs/heads/master@{#454564}

[add] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-and-text-repaint-expected.html
[add] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-and-text-repaint.html
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/layout/svg/LayoutSVGRoot.cpp
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/c63e571885b21aa510dc323ecb1c5bc18f1ed29b/third_party/WebKit/Source/core/style/StyleDifference.h

Comment 5 by r...@opera.com, Mar 3 2017

Status: Started (was: Duplicate)
De-duplicating since this was a regression from an attempt to fix the duplicate issue.

Comment 6 by r...@opera.com, Mar 3 2017

Status: Fixed (was: Started)

Comment 7 by r...@opera.com, Mar 3 2017

Owner: r...@opera.com

Comment 8 by r...@opera.com, Mar 24 2017

Blocking: 704576

Comment 9 by r...@opera.com, Mar 24 2017

Labels: -OS-Mac ReleaseBlock-Stable Merge-Request-58 M-58 OS-All
Requesting merge to 58 inheriting ReleaseBlock-Stable from blocked issue. This regression fix landed after the 58 branch point.

Project Member

Comment 10 by sheriffbot@chromium.org, Mar 24 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 24 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0

commit 5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0
Author: Rune Lillesveen <rune@opera.com>
Date: Fri Mar 24 10:46:04 2017

needsPaintInvalidation() should not return true for selection.

In [1] we introduced a paint invalidation constant for repainting
selection. needsPaintInvalidation() started to also return true when
only the selection needed paint invalidation. Callers of that method
assumes that true will cause at least a full element repaint which made
us skip setting the diff to invalidate the object and only invalidate
the selection in some cases. Instead, only return true for
PaintInvalidationObject and PaintInvalidationSubtree.

[1] https://crrev.com/eff357ef

R=mstensho@opera.com
BUG= 697337 

Review-Url: https://codereview.chromium.org/2727843004
Cr-Commit-Position: refs/heads/master@{#454564}
(cherry picked from commit c63e571885b21aa510dc323ecb1c5bc18f1ed29b)

Review-Url: https://codereview.chromium.org/2774763003 .
Cr-Commit-Position: refs/branch-heads/3029@{#406}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[add] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-and-text-repaint-expected.html
[add] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-and-text-repaint.html
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/layout/svg/LayoutSVGRoot.cpp
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/5bedd07e8a1c07c0b8cb94b02b9b38d7b81896e0/third_party/WebKit/Source/core/style/StyleDifference.h

Comment 12 by tkent@chromium.org, Mar 27 2017

 Issue 705204  has been merged into this issue.
Labels: TE-Verified-M58 TE-Verified-58.0.3029.41
Verified the fix on Mac 10.12.3, Win-10 and ubuntu 14.04 using Chrome beta version #58.0.3029.41 as per the comment #0.

Observed that the color of the anchor tag toggled when clicked as expected.

Hence, the fix is working as expected.

Attaching the screencast for reference

Adding the verified labels.

Thanks...!!
697337.mp4
992 KB View Download

Sign in to add a comment