Skip to content
This repository has been archived by the owner. It is now read-only.

Using RenderFrameHost::CopyImageAt for image copy #13108

Merged
merged 1 commit into from Feb 12, 2018
Merged

Using RenderFrameHost::CopyImageAt for image copy #13108

merged 1 commit into from Feb 12, 2018

Conversation

@darkdh
Copy link
Member

darkdh commented Feb 12, 2018

fix #7388

Auditors: @bbondy, @bsclifton

Test Plan:

  1. Go to https://www.brave.com/about/images/brave_icon_512x.png
  2. Click "Copy Image" from context menu
  3. The image should be copied to your clipboard instead of url
  4. Paste it to somewhere else like slack channel to verify

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header
fix #7388

Auditors: @bbondy, @bsclifton

Test Plan:
1. Go to https://www.brave.com/about/images/brave_icon_512x.png
2. Click "Copy Image" from context menu
3. The image should be copied to your clipboard instead of url
4. Paste it to somewhere else like slack channel to verify
@darkdh darkdh added this to the 0.21.x (Beta Channel) milestone Feb 12, 2018
@darkdh darkdh self-assigned this Feb 12, 2018
@darkdh darkdh requested review from bbondy and bsclifton Feb 12, 2018
@bsclifton bsclifton modified the milestones: 0.21.x (Beta Channel), 0.20.x Release 2 (Referral Promotion) Feb 12, 2018
@codecov-io
Copy link

codecov-io commented Feb 12, 2018

Codecov Report

Merging #13108 into master will increase coverage by 0.02%.
The diff coverage is 30.76%.

@@            Coverage Diff             @@
##           master   #13108      +/-   ##
==========================================
+ Coverage   56.31%   56.33%   +0.02%     
==========================================
  Files         280      278       -2     
  Lines       27591    27561      -30     
  Branches     4499     4498       -1     
==========================================
- Hits        15537    15526      -11     
+ Misses      12054    12035      -19
Flag Coverage Δ
#unittest 56.33% <30.76%> (+0.02%) ⬆️
Impacted Files Coverage Δ
js/constants/appConstants.js 100% <ø> (ø) ⬆️
js/stores/appStore.js 16.13% <ø> (+0.02%) ⬆️
app/browser/reducers/tabsReducer.js 41.9% <0%> (-0.62%) ⬇️
js/actions/appActions.js 19.78% <0%> (ø) ⬆️
js/contextMenus.js 18.53% <0%> (+0.08%) ⬆️
app/browser/tabs.js 24.21% <66.66%> (-0.15%) ⬇️
js/lib/imageUtil.js
Copy link
Member

petemill left a comment

This works great, I verified that an image is copied at it's 'natural' size rather than its rendered size, which is perfect 👌 . Tested by copying the image from https://codepen.io/anon/pen/bLRRWR

@petemill
Copy link
Member

petemill commented Feb 12, 2018

Just a note that when cherry-picking to 0.20.x, even though there will not be a merge error, we will need to change webContentsCache.getWebContents(tabId) to getWebContents(tabId).

@petemill petemill merged commit f5224a0 into master Feb 12, 2018
1 of 3 checks passed
1 of 3 checks passed
codecov/patch 30.76% of diff hit (target 56.31%)
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
codecov/project 56.33% (+0.02%) compared to a085fcd
Details
@petemill petemill deleted the issue-7388 branch Feb 12, 2018
petemill added a commit that referenced this pull request Feb 12, 2018
Using RenderFrameHost::CopyImageAt for image copy
petemill added a commit that referenced this pull request Feb 12, 2018
Using RenderFrameHost::CopyImageAt for image copy
petemill added a commit that referenced this pull request Feb 12, 2018
Using RenderFrameHost::CopyImageAt for image copy
@petemill
Copy link
Member

petemill commented Feb 12, 2018

0.22.x 7da7fcb
0.21.x 7b0b075
0.20.x ef0a205

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.