Skip to content

Commit

Permalink
allow reporting webcompat issues through the Shields panel
Browse files Browse the repository at this point in the history
  • Loading branch information
antonok-edm committed Sep 20, 2019
1 parent 23ea939 commit f9fff47
Show file tree
Hide file tree
Showing 19 changed files with 123 additions and 15 deletions.
25 changes: 25 additions & 0 deletions browser/extensions/api/brave_shields_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <utility>

#include "base/strings/string_number_conversions.h"
#include "brave/browser/brave_webcompat_reporter/webcompat_reporter_dialog.h"
#include "brave/common/extensions/api/brave_shields.h"
#include "brave/common/extensions/extension_constants.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
Expand Down Expand Up @@ -295,5 +296,29 @@ BraveShieldsGetNoScriptControlTypeFunction::Run() {
return RespondNow(OneArgument(std::move(result)));
}

ExtensionFunction::ResponseAction BraveShieldsReportBrokenSiteFunction::Run() {
std::unique_ptr<brave_shields::ReportBrokenSite::Params> params(
brave_shields::ReportBrokenSite::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params.get());

// Get web contents for this tab
content::WebContents* contents = nullptr;
if (!ExtensionTabUtil::GetTabById(
params->tab_id,
Profile::FromBrowserContext(browser_context()),
false,
nullptr,
nullptr,
&contents,
nullptr)) {
return RespondNow(Error(tabs_constants::kTabNotFoundError,
base::NumberToString(params->tab_id)));
}

OpenWebcompatReporterDialog(contents);

return RespondNow(NoArguments());
}

} // namespace api
} // namespace extensions
11 changes: 11 additions & 0 deletions browser/extensions/api/brave_shields_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ class BraveShieldsGetNoScriptControlTypeFunction
ResponseAction Run() override;
};

class BraveShieldsReportBrokenSiteFunction
: public UIThreadExtensionFunction {
public:
DECLARE_EXTENSION_FUNCTION("braveShields.reportBrokenSite", UNKNOWN)

protected:
~BraveShieldsReportBrokenSiteFunction() override {}

ResponseAction Run() override;
};

} // namespace api
} // namespace extensions

Expand Down
11 changes: 11 additions & 0 deletions common/extensions/api/brave_shields.json
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,17 @@
"parameters": []
}
]
},
{
"name": "reportBrokenSite",
"type": "function",
"description": "Bring up a modal for reporting a site broken by Shields to the Brave developers",
"parameters": [
{
"name": "tabId",
"type": "integer"
}
]
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
"description": "Message telling the user to disable shields if a site appears broken"
},
"disabledMessage": {
"message": "You’re browsing this site without any privacy and security protections.",
"description": "Message telling the user that shields are disabled"
"message": "You’re browsing this site without Brave's privacy and security protections. Does it not work right with Shields up?",
"description": "Message telling the user that shields are disabled, and prompting them towards reporting an issue if necessary"
},
"reportBrokenSite": {
"message": "Report a broken site",
"description": "Message for the button allowing users to report a broken site to Brave developers"
},
"itemsBlocked": {
"message": "Items blocked",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ export const shieldsToggled: actions.ShieldsToggled = (setting = 'allow') => {
}
}

export const reportBrokenSite: actions.ReportBrokenSite = () => {
return {
type: types.REPORT_BROKEN_SITE
}
}

export const resourceBlocked: actions.ResourceBlocked = (details) => {
return {
type: types.RESOURCE_BLOCKED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ export const setAllowScriptOriginsOnce = (origins: Array<string>, tabId: number)
})
})

/**
* Open a prompt that allows the user to submit a report telling Brave that the current website is broken by Shields.
* @param {number} tabId ID of the tab whose contents are being reported
*/
export const reportBrokenSite = (tabId: number) =>
chrome.braveShields.reportBrokenSite(tabId)

export type GetViewPreferencesData = {
showAdvancedView: boolean
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import {
setAllowCookies,
toggleShieldsValue,
requestShieldPanelData,
setAllowScriptOriginsOnce
setAllowScriptOriginsOnce,
reportBrokenSite
} from '../api/shieldsAPI'
import { reloadTab } from '../api/tabsAPI'

Expand Down Expand Up @@ -123,6 +124,16 @@ export default function shieldsPanelReducer (
.updateTabShieldsData(state, tabId, { braveShields: action.setting })
break
}
case shieldsPanelTypes.REPORT_BROKEN_SITE: {
const tabId: number = shieldsPanelState.getActiveTabId(state)
const tabData = shieldsPanelState.getActiveTabData(state)
if (!tabData) {
console.error('Active tab not found')
break
}
reportBrokenSite(tabId)
break
}
case shieldsPanelTypes.HTTPS_EVERYWHERE_TOGGLED: {
const tabData = shieldsPanelState.getActiveTabData(state)
if (!tabData) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export const DisabledContentText = styled<{}, 'div'>('div')`
font-size: 12px;
font-weight: normal;
line-height: 18px;
text-align: center;
`

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,11 +428,11 @@ export const BlockedListFooterWithOptions = styled<{}, 'footer'>('footer')`
export const DisabledContentView = styled<{}, 'section'>('section')`
box-sizing: border-box;
display: grid;
grid-template-columns: 2fr 5fr;
grid-gap: 4px;
align-items: center;
max-width: 80%;
margin: 5px auto 8px;
grid-template-rows: auto auto;
grid-gap: 12px;
justify-items: center;
max-width: 70%;
margin: 10px auto -5px;
`

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

export const SHIELDS_PANEL_DATA_UPDATED = 'SHIELDS_PANEL_DATA_UPDATED'
export const SHIELDS_TOGGLED = 'SHIELDS_TOGGLED'
export const REPORT_BROKEN_SITE = 'REPORT_BROKEN_SITE'
export const RESOURCE_BLOCKED = 'RESOURCE_BLOCKED'
export const BLOCK_ADS_TRACKERS = 'BLOCK_ADS_TRACKERS'
export const CONTROLS_TOGGLED = 'CONTROLS_TOGGLED'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TotalBlockedStatsNumber,
TotalBlockedStatsText,
DisabledContentView,
ShieldIcon,
ShieldsButton,
DisabledContentText,
Toggle
} from '../../components'
Expand All @@ -41,6 +41,7 @@ interface CommonProps {
hostname: string
isBlockedListOpen: boolean
shieldsToggled: ShieldsToggled
reportBrokenSite: () => void
}

interface BlockedItemsProps {
Expand Down Expand Up @@ -75,6 +76,11 @@ export default class Header extends React.PureComponent<Props, {}> {
this.props.shieldsToggled(shieldsOption)
}

onReportBrokenSite = () => {
this.props.reportBrokenSite()
window.close()
}

render () {
const { enabled, favicon, hostname, isBlockedListOpen } = this.props
return (
Expand Down Expand Up @@ -109,8 +115,8 @@ export default class Header extends React.PureComponent<Props, {}> {
)
: (
<DisabledContentView>
<div><ShieldIcon /></div>
<DisabledContentText>{getLocale('disabledMessage')}</DisabledContentText>
<ShieldsButton level='secondary' type='default' onClick={this.onReportBrokenSite} text={getLocale('reportBrokenSite')} />
</DisabledContentView>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { Tab, PersistentData } from '../../types/state/shieldsPannelState'
import { isShieldsEnabled, getFavicon } from '../../helpers/shieldsUtils'
import {
ShieldsToggled,
ReportBrokenSite,
BlockAdsTrackers,
HttpsEverywhereToggled,
BlockJavaScript,
Expand All @@ -38,6 +39,7 @@ import {
interface Props {
actions: {
shieldsToggled: ShieldsToggled
reportBrokenSite: ReportBrokenSite
blockAdsTrackers: BlockAdsTrackers
httpsEverywhereToggled: HttpsEverywhereToggled
blockJavaScript: BlockJavaScript
Expand Down Expand Up @@ -104,6 +106,7 @@ export default class Shields extends React.PureComponent<Props, State> {
scriptsBlocked={shieldsPanelTabData.javascriptBlocked}
fingerprintingBlocked={shieldsPanelTabData.fingerprintingBlocked}
shieldsToggled={actions.shieldsToggled}
reportBrokenSite={actions.reportBrokenSite}
/>
{
this.isShieldsEnabled && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as shieldsAPI from '../background/api/shieldsAPI'
import { Tab, PersistentData } from '../types/state/shieldsPannelState'
import {
ShieldsToggled,
ReportBrokenSite,
BlockAdsTrackers,
HttpsEverywhereToggled,
BlockJavaScript,
Expand All @@ -35,6 +36,7 @@ import {
interface Props {
actions: {
shieldsToggled: ShieldsToggled
reportBrokenSite: ReportBrokenSite
blockAdsTrackers: BlockAdsTrackers
httpsEverywhereToggled: HttpsEverywhereToggled
blockJavaScript: BlockJavaScript
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
TotalBlockedStatsNumber,
TotalBlockedStatsText,
DisabledContentView,
ShieldIcon,
ShieldsButton,
DisabledContentText,
Toggle
} from '../../components'
Expand All @@ -42,6 +42,7 @@ interface CommonProps {
hostname: string
shieldsToggled: ShieldsToggled
toggleReadOnlyView: () => void
reportBrokenSite: () => void
}

interface BlockedItemsProps {
Expand Down Expand Up @@ -71,6 +72,11 @@ export default class Header extends React.PureComponent<Props, {}> {
this.props.shieldsToggled(shieldsOption)
}

onReportBrokenSite = () => {
this.props.reportBrokenSite()
window.close()
}

render () {
const { enabled, favicon, hostname, toggleReadOnlyView } = this.props
return (
Expand Down Expand Up @@ -106,8 +112,8 @@ export default class Header extends React.PureComponent<Props, {}> {
)
: (
<DisabledContentView>
<div><ShieldIcon /></div>
<DisabledContentText>{getLocale('disabledMessage')}</DisabledContentText>
<ShieldsButton level='secondary' type='default' size='small' onClick={this.onReportBrokenSite} text={getLocale('reportBrokenSite')} />
</DisabledContentView>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Footer from './footer'

import {
ShieldsToggled,
ReportBrokenSite,
BlockAdsTrackers,
HttpsEverywhereToggled,
BlockJavaScript,
Expand All @@ -33,6 +34,7 @@ import { getFavicon, isShieldsEnabled } from '../../helpers/shieldsUtils'
interface Props {
actions: {
shieldsToggled: ShieldsToggled
reportBrokenSite: ReportBrokenSite
blockAdsTrackers: BlockAdsTrackers
httpsEverywhereToggled: HttpsEverywhereToggled
blockJavaScript: BlockJavaScript
Expand Down Expand Up @@ -77,6 +79,7 @@ export default class ShieldsSimpleView extends React.PureComponent<Props, {}> {
fingerprintingBlocked={shieldsPanelTabData.fingerprintingBlocked}
shieldsToggled={actions.shieldsToggled}
toggleReadOnlyView={toggleReadOnlyView}
reportBrokenSite={actions.reportBrokenSite}
/>
<Footer
enabled={this.isShieldsEnabled}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

import ITheme from 'brave-ui/theme/theme-interface'
import defaultTheme from 'brave-ui/theme/brave-default'
import darkTheme from 'brave-ui/theme/brave-dark'
import colors from 'brave-ui/theme/colors'

const shieldsDarkTheme: ITheme = {
...defaultTheme,
...darkTheme,
name: 'Shields Dark',
color: {
...defaultTheme.color,
...darkTheme.color,
lionLogo: colors.grey700,
text: colors.white,
panelBackground: '#17171F',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ export interface ShieldsToggled {
(setting: BlockOptions): ShieldsToggledReturn
}

interface ReportBrokenSiteReturn {
type: types.REPORT_BROKEN_SITE
}

export interface ReportBrokenSite {
(): ReportBrokenSiteReturn
}

interface ResourceBlockedReturn {
type: types.RESOURCE_BLOCKED
details: BlockDetails
Expand Down Expand Up @@ -159,6 +167,7 @@ export interface SetAdvancedViewFirstAccess {
export type shieldPanelActions =
ShieldsPanelDataUpdatedReturn |
ShieldsToggledReturn |
ReportBrokenSiteReturn |
ResourceBlockedReturn |
BlockAdsTrackersReturn |
ControlsToggledReturn |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as types from '../../constants/shieldsPanelTypes'

export type SHIELDS_PANEL_DATA_UPDATED = typeof types.SHIELDS_PANEL_DATA_UPDATED
export type SHIELDS_TOGGLED = typeof types.SHIELDS_TOGGLED
export type REPORT_BROKEN_SITE = typeof types.REPORT_BROKEN_SITE
export type RESOURCE_BLOCKED = typeof types.RESOURCE_BLOCKED
export type BLOCK_ADS_TRACKERS = typeof types.BLOCK_ADS_TRACKERS
export type CONTROLS_TOGGLED = typeof types.CONTROLS_TOGGLED
Expand Down
1 change: 1 addition & 0 deletions components/definitions/chromel.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ declare namespace chrome.braveShields {
const getHTTPSEverywhereEnabledAsync: any
const setNoScriptControlTypeAsync: any
const getNoScriptControlTypeAsync: any
const reportBrokenSite: any

type BraveShieldsViewPreferences = {
showAdvancedView: boolean
Expand Down

0 comments on commit f9fff47

Please sign in to comment.