|
|
Created:
3 years, 8 months ago by hayato Modified:
3 years, 7 months ago Reviewers:
Alexander Alekseev, achuithb, kochi CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, kinuko+watch, rwlbuis Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake /deep/ as no-op and remove ::shadow in dynamic profile
Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns
To make this CL small one, and easy to be reverted, all tests which depend on /deep/
(or ::shadow) were either removed or updated in another CL:
https://bugs.chromium.org/p/chromium/issues/detail?id=715034.
This CL only touched the small part so that users can't use /deep/ or ::shadow
in CSS dynamic profile in M60. The further internal clean up is needed in other CLs.
BUG=489954
Review-Url: https://codereview.chromium.org/2778983006
Cr-Commit-Position: refs/heads/master@{#471684}
Committed: https://chromium.googlesource.com/chromium/src/+/a7ab8a110bd6b5339c03a34a5a6bb9a419a49e60
Patch Set 1 #Patch Set 2 : rebase; try #Patch Set 3 : try #Patch Set 4 : fix more tests #Patch Set 5 : rebased #Patch Set 6 : rebase #Patch Set 7 : rebase #
Total comments: 7
Patch Set 8 : update #Patch Set 9 : rebased #
Created: 3 years, 7 months ago
Messages
Total messages: 66 (50 generated)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
rebase; try
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
try
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
fix more tests
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
rebased
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
rebase
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
rebase
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns BUG=489954 ========== to ========== (Don't land until M60) Make /deep/ as no-op in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns All tests which depend on /deep/ (or ::shadow) are either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034 BUG=489954 ==========
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns All tests which depend on /deep/ (or ::shadow) are either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034 BUG=489954 ========== to ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL smaller, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further clean up is needed in other CLs. BUG=489954 ==========
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL smaller, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further clean up is needed in other CLs. BUG=489954 ========== to ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further clean up is needed in other CLs. BUG=489954 ==========
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further clean up is needed in other CLs. BUG=489954 ========== to ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further internal clean up is needed in other CLs. BUG=489954 ==========
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only fixed the Web API surface so that users can't use /deep/ or ::shadow in CSS dynamic profile. The further internal clean up is needed in other CLs. BUG=489954 ========== to ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only touched the small part so that users can't use /deep/ or ::shadow in CSS dynamic profile in M60. The further internal clean up is needed in other CLs. BUG=489954 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
hayato@chromium.org changed reviewers: + achuith@chromium.org, alemate@chromium.org, kochi@chromium.org
PTAL I think the CL is ready to be reviewed. +achuith, alemate Regarding bug 719331, I am going to land this CL. Could you confirm whether this regress ChromeOS Login UI or not?
https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSSelector.h (right): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSSelector.h:137: kShadowDeepAsDescendant, // /deep/ as an alias for descendant nit: moving this next to kShadowDeep? https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp (right): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:815: // TODO: Check second_compound->GetPseudoType() What does this mean? https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h (left): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h:69: static std::unique_ptr<CSSParserSelector> This change is not necessary for this CL? (You removed some changes in .cpp?)
update
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks you for the review. https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/CSSSelector.h (right): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/CSSSelector.h:137: kShadowDeepAsDescendant, // /deep/ as an alias for descendant Done https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp (right): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:815: // TODO: Check second_compound->GetPseudoType() This was my private TODO which I forgot to remove. https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h (left): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h:69: static std::unique_ptr<CSSParserSelector> Done. It looks I forgot to revert this header.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp (right): https://codereview.chromium.org/2778983006/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:815: // TODO: Check second_compound->GetPseudoType() On 2017/05/10 06:57:04, hayato wrote: > This was my private TODO which I forgot to remove. Acknowledged.
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== (Don't land until M60) Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only touched the small part so that users can't use /deep/ or ::shadow in CSS dynamic profile in M60. The further internal clean up is needed in other CLs. BUG=489954 ========== to ========== Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only touched the small part so that users can't use /deep/ or ::shadow in CSS dynamic profile in M60. The further internal clean up is needed in other CLs. BUG=489954 ==========
rebased
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by hayato@chromium.org
The CQ bit was checked by hayato@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org Link to the patchset: https://codereview.chromium.org/2778983006/#ps160001 (title: "rebased")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 160001, "attempt_start_ts": 1494821636928600, "parent_rev": "9937b524e266fe058f637e442f96efba14b35c1f", "commit_rev": "a7ab8a110bd6b5339c03a34a5a6bb9a419a49e60"}
Message was sent while issue was closed.
Description was changed from ========== Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only touched the small part so that users can't use /deep/ or ::shadow in CSS dynamic profile in M60. The further internal clean up is needed in other CLs. BUG=489954 ========== to ========== Make /deep/ as no-op and remove ::shadow in dynamic profile Intent to Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/HX5Y8Ykr5Ns To make this CL small one, and easy to be reverted, all tests which depend on /deep/ (or ::shadow) were either removed or updated in another CL: https://bugs.chromium.org/p/chromium/issues/detail?id=715034. This CL only touched the small part so that users can't use /deep/ or ::shadow in CSS dynamic profile in M60. The further internal clean up is needed in other CLs. BUG=489954 Review-Url: https://codereview.chromium.org/2778983006 Cr-Commit-Position: refs/heads/master@{#471684} Committed: https://chromium.googlesource.com/chromium/src/+/a7ab8a110bd6b5339c03a34a5a6b... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://chromium.googlesource.com/chromium/src/+/a7ab8a110bd6b5339c03a34a5a6b...
Message was sent while issue was closed.
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2885163002/ by rkc@google.com. The reason for reverting is: Completely breaks Chrome OS login UI.
Message was sent while issue was closed.
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2885153003/ by rkc@chromium.org. The reason for reverting is: (from the right account this time) Completely breaks Chrome OS login UI. |