Add extension inspect views #10163

evq · 2017-07-27T01:47:42Z

Fixes #8012

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.

Test Plan:

Reviewer Checklist:

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

bridiver · 2017-07-27T04:52:00Z

js/about/extensions.js

+    chrome.tabs.query(
+      {currentWindow: true, active: true},
+      function (tabArray) {
+        chrome.ipcRenderer.send('load-url-requested', tabArray[0].id, 'chrome-extension://' + extensionId + '/_generated_background_page.html')


I'm not sure why this ipc message even exists, but please use appActions.loadURLRequested instead. Sending ipc messages directly is deprecated. However, I don't think this will work correctly in either case because it will create a new background page instead of opening the existing one

bridiver

see comments

NejcZdovc · 2017-07-27T06:27:02Z

less/about/common.less

@@ -20,3 +20,22 @@ html, body, #appContainer, #appContainer > div {
 body {
  font-size: 100%;
 }
+
+.linkText {


Can you please use Aphrodite, so move this css into js file. More info here: https://github.com/brave/browser-laptop/wiki/Refactoring-styles-to-Aphrodite. Thank you 😃

@evq ping me if you want me to do

evq · 2017-07-29T00:44:24Z

@bridiver @NejcZdovc I uploaded a second commit which should address both of your comments :)

luixxiul · 2017-07-29T06:39:24Z

app/renderer/components/styles/commonStyles.js

+    color: globalStyles.color.braveOrange,
+    cursor: 'pointer',
+    margin: '0',
+    ':hover': {


@evq could you add a blank line between margin and :hover?

NejcZdovc

@evq Please squash commits and add auto closing tag to the new commit, other than that it looks good to me. Also please provide test plan for it

luixxiul · 2017-07-29T06:41:09Z

app/renderer/components/styles/commonStyles.js

+    }
+  },
+
+  linkTextSmall: {


@evq would you please change linkTextSmall into linkText_small based on our guideline, which is available here? https://github.com/brave/browser-laptop/blob/master/docs/style.md#defining-our-blocks-elements-and-modifiers thanks!

NejcZdovc · 2017-07-29T06:44:53Z

@evq if I click on Torrent Viewer there is no dev tools opened, I just get normal blank tab with this url chrome-extension://fmdpfempfmekjkcfdehndghogpnpjeno/_generated_background_page.html. Is this correct?

evq · 2017-07-30T09:06:30Z

@NejcZdovc Weird, thanks for testing! I don't seem to be able to get the dev tools to open by manually typing the url for the torrent viewer extension either. It doesn't seem like the manifest specifies a background page, so I would expect _generated_background_page.html to work. I will have to dig deeper.

luixxiul · 2017-07-30T09:23:24Z

js/about/preferences.js

@@ -607,15 +607,15 @@ class SecurityTab extends ImmutableComponent {
        </SettingItem>
        {
          getSetting(settings.ACTIVE_PASSWORD_MANAGER, this.props.settings) === passwordManagers.BUILT_IN
-          ? <label className='linkTextSmall' data-l10n-id='managePasswords'
+          ? <label className={css(commonStyles.linkText, commonStyles.linkTextSmall)} data-l10n-id='managePasswords'


@evq please update this as well.

oops! same issue in js/about/extensions.js as well

luixxiul · 2017-07-30T09:23:32Z

js/about/preferences.js

            onClick={aboutActions.createTabRequested.bind(null, {
              url: 'about:passwords'
            })} />
          : null
        }
        {
          getSetting(settings.ACTIVE_PASSWORD_MANAGER, this.props.settings) === passwordManagers.LAST_PASS
-          ? <label className='linkTextSmall' data-l10n-id='preferences'
+          ? <label className={css(commonStyles.linkText, commonStyles.linkTextSmall)} data-l10n-id='preferences'


@evq ditto.

evq · 2017-07-30T21:28:09Z

@NejcZdovc I have made changes such that the inspect views are not shown if the extension is disabled or does not specify background scripts or pages (this was the case for torrent viewer).

All comments should be addressed at this point.

codecov-io · 2017-07-31T02:38:49Z

Codecov Report

Merging #10163 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #10163      +/-   ##
==========================================
+ Coverage   53.67%   53.67%   +<.01%     
==========================================
  Files         238      238              
  Lines       21049    21053       +4     
  Branches     3256     3256              
==========================================
+ Hits        11297    11301       +4     
  Misses       9752     9752

Flag	Coverage Δ
#unittest	`53.67% <100%> (ø)`	⬆️

Impacted Files	Coverage Δ
app/renderer/components/styles/commonStyles.js	`100% <ø> (ø)`	⬆️
js/about/preferences.js	`42.91% <ø> (ø)`	⬆️
.../components/preferences/payment/disabledContent.js	`100% <100%> (ø)`	⬆️
app/renderer/components/preferences/pluginsTab.js	`49.01% <100%> (+3.18%)`	⬆️

luixxiul

The test plan works.

@luixxiul

Fixes #8012 Auditors: @luixxiul Test Plan: Ensure extension links work when presented and are not present for disabled / invalid: 1. Go to `about:extensions` 2. Click on the pdf viewer inspect view and confirm dev tools are launched 3. In another tab go to `about:preferences#extensions` and enable 1password 4. Click on the 1password inspect view and confirm dev tools are launched 5. In another tab go to `about:preferences#extensions` and enable bitwarden 6. Confirm the dev tools pane for 1password has closed 7. Confirm that `about:extensions` no longer shows inspect views for 1password 8. Click on the bitwarden inspect view and confirm dev tools are launched 9. Confirm that `about:extensions` has no inspect views for torrent viewer Ensure the following appear as orange anchor links: 1. Payment FAQ on `about:preferences#payment` 2. Adobe and wiki `about:preferences#plugins` 3. Release notes and revision on `about:brave` 4. Inspect pages on `about:extensions` 5. Manage passwords on `about:preferences#security`

luixxiul · 2017-08-10T06:47:47Z

@bridiver would you please review the change again and make sure your comments were addressed? thanks!

bsclifton · 2017-08-22T20:59:01Z

@bridiver can you please review (per the feedback above). Thanks! 😄

bridiver

++

bridiver reviewed Jul 27, 2017

View reviewed changes

bridiver suggested changes Jul 27, 2017

View reviewed changes

NejcZdovc reviewed Jul 27, 2017

View reviewed changes

evq changed the title ~~Add extension inspect views (_generated_background_page.html only)~~ Add extension inspect views Jul 29, 2017

luixxiul reviewed Jul 29, 2017

View reviewed changes

NejcZdovc approved these changes Jul 29, 2017

View reviewed changes

luixxiul reviewed Jul 29, 2017

View reviewed changes

NejcZdovc assigned evq Jul 29, 2017

luixxiul reviewed Jul 30, 2017

View reviewed changes

evq force-pushed the inspect_views branch from 1e3423d to 0cb6c16 Compare July 30, 2017 21:25

luixxiul added feature/developer-tools feature/extensions labels Jul 31, 2017

luixxiul force-pushed the inspect_views branch from c270b07 to eb540c4 Compare July 31, 2017 02:22

luixxiul force-pushed the inspect_views branch from eb540c4 to 5d85d0f Compare July 31, 2017 02:48

luixxiul self-requested a review July 31, 2017 03:35

evq force-pushed the inspect_views branch from 5d85d0f to daad8a5 Compare July 31, 2017 06:47

luixxiul reviewed Jul 31, 2017

View reviewed changes

luixxiul added this to the 0.21.x (Nightly Channel) milestone Jul 31, 2017

evq force-pushed the inspect_views branch from daad8a5 to 8110f82 Compare August 9, 2017 08:15

luixxiul requested a review from bridiver August 10, 2017 06:47

bridiver approved these changes Aug 28, 2017

View reviewed changes

luixxiul merged commit a2d9ecb into master Aug 31, 2017

bsclifton deleted the inspect_views branch September 4, 2017 07:12

bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017

luixxiul mentioned this pull request Nov 17, 2017

Hyperlinks in about:brave always looks grey #12015

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add extension inspect views #10163

Add extension inspect views #10163

evq commented Jul 27, 2017 •

edited by bsclifton

Loading

bridiver Jul 27, 2017

bridiver left a comment

NejcZdovc Jul 27, 2017

luixxiul Jul 27, 2017

evq commented Jul 29, 2017

luixxiul Jul 29, 2017

NejcZdovc left a comment •

edited

Loading

luixxiul Jul 29, 2017

NejcZdovc commented Jul 29, 2017

evq commented Jul 30, 2017

luixxiul Jul 30, 2017

evq Jul 30, 2017

luixxiul Jul 30, 2017

evq commented Jul 30, 2017

codecov-io commented Jul 31, 2017 •

edited

Loading

luixxiul left a comment

luixxiul commented Aug 10, 2017 •

edited

Loading

bsclifton commented Aug 22, 2017

bridiver left a comment

Add extension inspect views #10163

Add extension inspect views #10163

Conversation

evq commented Jul 27, 2017 • edited by bsclifton Loading

Choose a reason for hiding this comment

bridiver left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evq commented Jul 29, 2017

Choose a reason for hiding this comment

NejcZdovc left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NejcZdovc commented Jul 29, 2017

evq commented Jul 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evq commented Jul 30, 2017

codecov-io commented Jul 31, 2017 • edited Loading

Codecov Report

luixxiul left a comment

Choose a reason for hiding this comment

luixxiul commented Aug 10, 2017 • edited Loading

bsclifton commented Aug 22, 2017

bridiver left a comment

Choose a reason for hiding this comment

evq commented Jul 27, 2017 •

edited by bsclifton

Loading

NejcZdovc left a comment •

edited

Loading

codecov-io commented Jul 31, 2017 •

edited

Loading

luixxiul commented Aug 10, 2017 •

edited

Loading