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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2905763003: Rollback ContextMenu (Closed)

Created:
3 years, 7 months ago by yuzuchan
Modified:
3 years, 6 months ago
Reviewers:
tkent, dglazkov
CC:
asvitkine+watch_chromium.org, blink-reviews, blink-reviews-frames_chromium.org, blink-reviews-html_chromium.org, blink-reviews-w3ctests_chromium.org, chromium-reviews, dglazkov+blink, kinuko+watch, loading-reviews+parser_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Rollback ContextMenu This CL aims to drop the context menu feature entirely. <menuitem> tag can be no longer used and <menu type='context'>, <menu label='...'> are no longer supported. BUG=87553 Review-Url: https://codereview.chromium.org/2905763003 Cr-Commit-Position: refs/heads/master@{#476230} Committed: https://chromium.googlesource.com/chromium/src/+/e90a0487386316a1e97f69159f20fbe5933cb3b9

Patch Set 1 #

Patch Set 2 : Delete a context menu condition #

Patch Set 3 : Remove unnecessary changes #

Patch Set 4 : Rewrite test results #

Patch Set 5 : Delete RuntimeEnabledFeatures::contextMenuEnabled #

Total comments: 12

Patch Set 6 : Rollback wpt test changes & Delete RelatedEvent #

Patch Set 7 : Modify layout tests #

Patch Set 8 : Removed icon & radiogroup #

Total comments: 2

Patch Set 9 : Delete icon-related code from ContextMenuItem class #

Total comments: 1

Patch Set 10 : Fix layout test results #

Patch Set 11 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+364 lines, -1933 lines) Patch
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG View 1 2 3 4 5 6 7 8 9 10 7 chunks +0 lines, -9 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/custom-elements/reactions/HTMLElement-expected.txt View 1 2 3 6 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-misc-expected.txt View 1 2 3 4 5 2 chunks +347 lines, -9 lines 0 comments Download
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/interactive-elements/the-menu-element/menuitem-label-expected.txt View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/html/semantics/interfaces-expected.txt View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/dom/HTMLElement/contextmenu.html View 1 chunk +0 lines, -146 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/dom/HTMLElement/contextmenu-expected.txt View 1 chunk +0 lines, -65 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/boolean-attribute-reflection.html View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/boolean-attribute-reflection-expected.txt View 1 chunk +0 lines, -12 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/domstring-attribute-reflection.html View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/domstring-attribute-reflection-expected.txt View 1 2 1 chunk +0 lines, -60 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/constructors/related-event-constructor.html View 1 2 3 4 5 6 1 chunk +0 lines, -56 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/constructors/related-event-constructor-expected.txt View 1 2 3 4 5 6 1 chunk +0 lines, -32 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/relatedevent.html View 1 2 3 4 5 6 1 chunk +0 lines, -23 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/events/relatedevent-expected.txt View 1 2 3 4 5 6 1 chunk +0 lines, -13 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/parser/parse-menuitem.html View 1 chunk +0 lines, -13 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/parser/parse-menuitem-expected.txt View 1 1 chunk +0 lines, -13 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/custom-context-menu.html View 1 chunk +0 lines, -200 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/custom-context-menu-expected.txt View 1 chunk +0 lines, -43 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menu-type-context.html View 1 chunk +0 lines, -10 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menu-type-context-expected.html View 1 chunk +0 lines, -11 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-click.html View 1 chunk +0 lines, -72 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-click-expected.txt View 1 chunk +0 lines, -27 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-crash-asan.html View 1 chunk +0 lines, -25 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-crash-asan-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-with-end-tag.html View 1 chunk +0 lines, -5 lines 0 comments Download
D third_party/WebKit/LayoutTests/html/menu_menuitem/menuitem-with-end-tag-expected.html View 1 chunk +0 lines, -7 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/misc/custom-context-menu.html View 1 chunk +0 lines, -206 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/misc/custom-context-menu-expected.txt View 1 chunk +0 lines, -45 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 4 chunks +0 lines, -27 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/element-instance-property-listing-expected.txt View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/element-instance-property-listing-expected.txt View 1 2 3 3 chunks +0 lines, -12 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 4 chunks +0 lines, -27 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8DevToolsHostCustom.cpp View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/core_idl_files.gni View 1 2 3 4 5 6 3 chunks +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/events/BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -2 lines 0 comments Download
D third_party/WebKit/Source/core/events/RelatedEvent.h View 1 2 3 4 5 1 chunk +0 lines, -49 lines 0 comments Download
D third_party/WebKit/Source/core/events/RelatedEvent.cpp View 1 2 3 4 5 1 chunk +0 lines, -41 lines 0 comments Download
M third_party/WebKit/Source/core/events/RelatedEvent.idl View 1 2 3 4 5 1 chunk +0 lines, -12 lines 0 comments Download
D third_party/WebKit/Source/core/events/RelatedEventInit.idl View 1 2 3 4 5 1 chunk +0 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/html/BUILD.gn View 1 2 3 4 5 6 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLAttributeNames.json5 View 1 2 3 4 5 6 7 3 chunks +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLElement.h View 1 2 3 4 2 chunks +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLElement.cpp View 1 2 3 4 5 2 chunks +0 lines, -44 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLElement.idl View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMenuElement.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMenuElement.cpp View 1 2 3 1 chunk +0 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMenuElement.idl View 1 chunk +0 lines, -3 lines 0 comments Download
D third_party/WebKit/Source/core/html/HTMLMenuItemElement.h View 1 chunk +0 lines, -34 lines 0 comments Download
D third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp View 1 chunk +0 lines, -83 lines 0 comments Download
D third_party/WebKit/Source/core/html/HTMLMenuItemElement.idl View 1 chunk +0 lines, -18 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLTagNames.json5 View 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLUnknownElement.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLUnknownElement.cpp View 1 2 3 4 5 1 chunk +0 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutTheme.cpp View 1 2 3 4 5 1 chunk +1 line, -4 lines 0 comments Download
M third_party/WebKit/Source/core/page/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/page/ContextMenuController.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/ContextMenuController.cpp View 2 chunks +0 lines, -30 lines 0 comments Download
D third_party/WebKit/Source/core/page/ContextMenuControllerTest.cpp View 1 2 3 4 5 1 chunk +0 lines, -148 lines 0 comments Download
D third_party/WebKit/Source/core/page/CustomContextMenuProvider.h View 1 chunk +0 lines, -48 lines 0 comments Download
D third_party/WebKit/Source/core/page/CustomContextMenuProvider.cpp View 1 chunk +0 lines, -149 lines 0 comments Download
M third_party/WebKit/Source/platform/ContextMenuItem.h View 1 2 3 4 5 6 7 8 3 chunks +0 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/platform/ContextMenuItem.cpp View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/web/ContextMenuClientImpl.cpp View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M tools/metrics/histograms/enums.xml View 1 2 3 4 5 6 7 8 9 10 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 54 (43 generated)
yuzuchan
PTAL This is how I feel about this patch: http://i.imgur.com/EXJUNjk.gif
3 years, 7 months ago (2017-05-26 04:24:49 UTC) #13
tkent
Also, I think we can remove |icon|-related stuff in ContextMenuItem class. https://codereview.chromium.org/2905763003/diff/80001/third_party/WebKit/LayoutTests/external/wpt/html/dom/elements-misc.js File third_party/WebKit/LayoutTests/external/wpt/html/dom/elements-misc.js (left): ...
3 years, 7 months ago (2017-05-26 04:48:54 UTC) #14
yuzuchan
Thanks for the review, PTAL again :) This is how I feel about this patch: ...
3 years, 6 months ago (2017-05-30 07:15:20 UTC) #30
tkent
The new patch set doesn't address my previous comment: > Also, I think we can ...
3 years, 6 months ago (2017-05-30 07:40:22 UTC) #31
yuzuchan
Thanks for the review again, and PTAL again :) This is how I feel about ...
3 years, 6 months ago (2017-05-30 08:43:18 UTC) #32
yuzuchan
Thanks for the review again, and PTAL again :) This is how I feel about ...
3 years, 6 months ago (2017-05-30 08:43:20 UTC) #33
tkent
+dglazkov FYI. lgtm https://codereview.chromium.org/2905763003/diff/180001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp File third_party/WebKit/Source/web/ContextMenuClientImpl.cpp (left): https://codereview.chromium.org/2905763003/diff/180001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#oldcode532 third_party/WebKit/Source/web/ContextMenuClientImpl.cpp:532: output_item.icon = input_item->Icon(); We can remove ...
3 years, 6 months ago (2017-05-30 08:49:41 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2905763003/200001
3 years, 6 months ago (2017-06-01 02:13:13 UTC) #46
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/452834)
3 years, 6 months ago (2017-06-01 02:21:54 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2905763003/220001
3 years, 6 months ago (2017-06-01 07:09:06 UTC) #51
commit-bot: I haz the power
3 years, 6 months ago (2017-06-01 09:02:57 UTC) #54
Message was sent while issue was closed. 
Committed patchset #11 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/e90a0487386316a1e97f69159f20...

Powered by Google App Engine
This is Rietveld 408576698