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

Issue 1389273003: 'View source code' option should be disabled in context menu for 'Paper Toolbar' of 'Pdf Viewer" (Closed)

Created:
5 years, 2 months ago by Deepak
Modified:
5 years, 2 months ago
Reviewers:
AKV, lazyboy
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

When we have viewing in a MimeHandlerViewGuest then 'View Source Code' will be disabled. This is needed as when context menu comes on the meterial UI buttons like On the 'Zoom-out','Zoom-in','Fullscreen' icon or 'Paper Toolbar' of the pdf viewer then these elements are not HTMLEmbedElements. So data.mediaType and data.mediaFlags values are not updated for these materiel UI toolbar. BUG=539269 Committed: https://crrev.com/400dbfa3846a40649f04f75dd65f92917e1e3a45 Cr-Commit-Position: refs/heads/master@{#353982}

Patch Set 1 #

Patch Set 2 : #

Total comments: 3

Patch Set 3 : Changes as per review comments. #

Patch Set 4 : Changes as per review comments. #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M chrome/browser/renderer_context_menu/render_view_context_menu.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (3 generated)
Deepak
For Peer review PTAL. Thanks
5 years, 2 months ago (2015-10-08 11:09:17 UTC) #2
AKV
peer review looks good to me!
5 years, 2 months ago (2015-10-08 13:45:55 UTC) #3
Deepak
PTAL
5 years, 2 months ago (2015-10-08 13:48:04 UTC) #5
Deepak
@lazyboy PTAL Thanks
5 years, 2 months ago (2015-10-08 13:48:41 UTC) #6
lazyboy
https://chromiumcodereview.appspot.com/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right): https://chromiumcodereview.appspot.com/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc#newcode1157 chrome/browser/renderer_context_menu/render_view_context_menu.cc:1157: return !extensions::MimeHandlerViewGuest::FromWebContents( Wouldn't this start showing view source for ...
5 years, 2 months ago (2015-10-08 18:35:39 UTC) #7
Deepak
https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right): https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc#newcode1157 chrome/browser/renderer_context_menu/render_view_context_menu.cc:1157: return !extensions::MimeHandlerViewGuest::FromWebContents( On 2015/10/08 18:35:39, lazyboy wrote: > Wouldn't ...
5 years, 2 months ago (2015-10-09 05:29:05 UTC) #8
lazyboy
https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right): https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc#newcode1157 chrome/browser/renderer_context_menu/render_view_context_menu.cc:1157: return !extensions::MimeHandlerViewGuest::FromWebContents( On 2015/10/09 05:29:05, Deepak wrote: > On ...
5 years, 2 months ago (2015-10-09 15:46:38 UTC) #9
Deepak
5 years, 2 months ago (2015-10-11 07:07:21 UTC) #10
Deepak
On 2015/10/09 15:46:38, lazyboy wrote: > https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc > File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right): > > https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc#newcode1157 > ...
5 years, 2 months ago (2015-10-11 07:15:56 UTC) #11
lazyboy
On 2015/10/11 07:15:56, Deepak wrote: > On 2015/10/09 15:46:38, lazyboy wrote: > > > https://codereview.chromium.org/1389273003/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc ...
5 years, 2 months ago (2015-10-13 17:36:48 UTC) #12
Deepak
@lazyboy Test case for current scenario requires: 1)Launching of Browser that have "PDF Viewer Extension ...
5 years, 2 months ago (2015-10-14 04:46:17 UTC) #13
lazyboy
On 2015/10/14 04:46:17, Deepak wrote: > @lazyboy > Test case for current scenario requires: > ...
5 years, 2 months ago (2015-10-14 07:38:36 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1389273003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1389273003/80001
5 years, 2 months ago (2015-10-14 07:39:35 UTC) #16
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 2 months ago (2015-10-14 08:29:30 UTC) #17
commit-bot: I haz the power
5 years, 2 months ago (2015-10-14 08:30:18 UTC) #18
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/400dbfa3846a40649f04f75dd65f92917e1e3a45
Cr-Commit-Position: refs/heads/master@{#353982}

Powered by Google App Engine
This is Rietveld 408576698