Page MenuHomePhabricator

Remove all inline styles to support a "style-src 'self'/<cdn-domain>" Content-Security-Policy
Open, LowPublic

Description

See T4340. See PHI399. A weakness of the current Content-Security-Policy is that we specify style-src ... 'unsafe-inline' because we use inline style attributes and one inline <style> tag. Ideally, we'd remove all uses of both style="..." and <style> and then tighten the Content-Security-Policy.

This is probably a long road.


Uses of <style>: We use one <style> tag to implement the "Monospaced Font Preference" setting. We could dynamically generate a custom external style sheet for each user's personal setting instead. This overlaps somewhat with T12311 and general dynamic generation of CSS variations.

We could also do this in Javascript, or remove the preference, or provide a set number of presets. It's possible that this setting isn't very useful, but on this install it looks like users have set it to about 80 distinct, legitimate values, which suggests it worth retaining support for.

Adding an extra per-user CSS request will imply some performance cost, but this should almost always hit caches and not be a big deal overall.


Uses of style="..."

Running git grep -i "'style'" gives me these, which are likely the majority:

Trivial/Lazy
// Rendering QR codes, color is always black or white and margin is fixed.
src/applications/auth/factor/PhabricatorTOTPAuthFactor.php:290:            'style' => 'background: '.$color,
src/applications/auth/factor/PhabricatorTOTPAuthFactor.php:300:        'style' => 'margin: 24px auto;',

// Password hash strength table.
src/applications/auth/provider/PhabricatorPasswordAuthProvider.php:28:        'style' => 'color: #009900',
src/applications/auth/provider/PhabricatorPasswordAuthProvider.php:35:        'style' => 'color: #990000',

src/applications/conduit/controller/PhabricatorConduitAPIController.php:595:      array('style' => 'white-space: pre-wrap;'),
src/applications/config/controller/PhabricatorConfigDatabaseController.php:29:          'style' => 'color: #aa0000;',

// DarkConsole
src/applications/console/plugin/DarkConsoleServicesPlugin.php:178:        phutil_tag('div', array('style' => 'clear: both;')),
src/applications/console/plugin/DarkConsoleXHProfPlugin.php:83:          'style' => 'float: right; margin: 1em 2em 0 0; font-weight: bold;',

src/applications/daemon/controller/PhabricatorDaemonLogViewController.php:165:          'style'   => 'width: 100%; height: 12em;',
src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php:139:        'style' => 'text-align: center;',
src/applications/diffusion/controller/DiffusionBrowseController.php:751:          'style' => 'display: inline-block;',

src/applications/files/controller/PhabricatorFileComposeController.php:87:          'style' => 'margin: 0 8px 8px 0',
src/applications/files/controller/PhabricatorFileComposeController.php:107:          'style' => 'margin: 0 8px 8px 0',
src/applications/fact/controller/PhabricatorFactChartController.php:66:        'style' => 'background: #ffffff; '.

src/applications/harbormaster/controller/HarbormasterUnitMessageViewController.php:95:          'style' =>

src/applications/maniphest/controller/ManiphestReportController.php:363:        'style' => 'border: 1px solid #BFCFDA; '.
src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php:165:          'style' => 'white-space: pre-wrap',
src/applications/metamta/replyhandler/PhabricatorMailTarget.php:83:              'style' => 'font-size: smaller; color: #92969D',
src/applications/multimeter/controller/MultimeterSampleController.php:294:        'style' => 'font-weight: bold',
src/applications/passphrase/view/PassphraseCredentialControl.php:125:          'style' => 'height: 20px;', // move aphront-form to tables
src/applications/phpast/controller/PhabricatorXHPASTViewFrameController.php:19:          'style'       => 'width: 100%; height: 800px;',
src/applications/releeph/view/request/ReleephRequestTypeaheadControl.php:30:        'style' => 'position: relative;',
src/applications/uiexample/examples/MacroEmojiExample.php:33:          'style' => 'width: 240px; height: 24px; float: left;',
src/applications/uiexample/examples/PHUITypeaheadExample.php:46:        'style' => 'padding: 8px',
src/applications/uiexample/examples/PhabricatorFilesComposeAvatarExample.php:70:          'style' => 'float: left;',
src/applications/uiexample/examples/PhabricatorGestureUIExample.php:35:        'style' => 'width: 320px; height: 240px; margin: auto;',
src/applications/uiexample/examples/PhabricatorMultiColumnUIExample.php:23:          'style' => 'border: 1px solid green;',
src/applications/uiexample/examples/PhabricatorMultiColumnUIExample.php:31:          'style' => 'border: 1px solid blue;',
src/applications/uiexample/examples/PhabricatorProjectBuiltinsExample.php:48:          'style' => 'float: left; padding: 4px;',
src/docs/user/configuration/custom_fields.diviner:193:          'style' => 'color: #ff00ff',
src/infrastructure/markup/rule/PhabricatorNavigationRemarkupRule.php:85:        'style' => 'color: #92969D;',
src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php:159:        'style' => 'background-color: #e7e7e7;
src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php:46:          'style'       => 'margin: 1em auto; border: 0px;',
src/view/control/AphrontTokenizerTemplateView.php:63:        'style'       => 'width: 0px;',
src/view/control/AphrontTokenizerTemplateView.php:70:    $content[] = phutil_tag('div', array('style' => 'clear: both;'), '');
src/view/control/AphrontTypeaheadTemplateView.php:64:        phutil_tag('div', array('style' => 'clear: both'), ''),
src/view/form/PHUIFormInsetView.php:69:          'style' => 'float: right;',
src/view/form/control/AphrontFormTypeaheadControl.php:30:        'style' => 'position: relative;',

These calls can be easily replaced with a class.

HTML Mail
src/applications/differential/editor/DifferentialTransactionEditor.php:1005:        'style' => implode(' ', $style),
src/applications/differential/editor/DifferentialTransactionEditor.php:1036:        'style' => implode(' ', $style),
src/applications/differential/editor/DifferentialTransactionEditor.php:1490:      array('style' => 'font-family: monospace;'), $patch);
src/applications/differential/mail/DifferentialInlineCommentMailView.php:206:          'style' => implode(' ', $style),
src/applications/differential/mail/DifferentialInlineCommentMailView.php:244:      'style' => 'padding: 0; margin: 8px;',
src/applications/differential/mail/DifferentialInlineCommentMailView.php:295:        'style' => $styles,
src/applications/differential/mail/DifferentialInlineCommentMailView.php:440:            'style' => implode(' ', $link_style),
src/applications/differential/mail/DifferentialMailView.php:18:        'style' => implode(' ', $style),
src/applications/differential/mail/DifferentialMailView.php:34:        'style' => implode(' ', $style),
src/applications/differential/mail/DifferentialMailView.php:43:        'style' => 'color: #4b4d51; font-weight: bold;',
src/applications/differential/mail/DifferentialMailView.php:57:        'style' => implode(' ', $style),
src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php:92:            'style' => $style,
src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php:101:            'style' => $context_style,
src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php:136:        'style' => 'color: #888888;',
src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php:143:      $style = $line['style'];
src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php:149:          'style' => $style,
src/applications/people/markup/PhabricatorMentionRemarkupRule.php:131:              'style' => $colors.'
src/applications/phid/remarkup/PhabricatorHandleRemarkupRule.php:83:              'style' => '
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2971:          'style' => implode(' ', $button_style),
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2988:        'style' => implode(' ', $xactions_style),
src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php:54:              'style' => $omit_styles,
src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php:62:              'style' => $old_styles,
src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php:70:              'style' => $new_styles,
src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php:92:        'style' => implode(' ', $styles),
src/infrastructure/markup/rule/PhabricatorKeyboardRemarkupRule.php:234:            'style' => $key_style,
src/infrastructure/markup/rule/PhabricatorKeyboardRemarkupRule.php:247:          'style' => $join_style,

These calls build HTML mail, which must use "style" so user agents will render the style. They do not need to be replaced.

Hiding Elements
src/applications/conpherence/view/ConpherenceDurableColumnView.php:115:      'style' => $style,
src/applications/conpherence/view/ConpherenceDurableColumnView.php:274:        'style' => 'display: none',
src/applications/conpherence/view/ConpherenceLayoutView.php:158:                'style' => 'display: none;',
src/applications/console/core/DarkConsoleCore.php:104:        'style' => $visible ? '' : 'display: none;',
src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php:110:          'style' => ($idx == $selected) ? null : 'display: none',
src/applications/drydock/view/DrydockRepositoryOperationStatusView.php:197:        'style' => 'display: none',
src/applications/drydock/view/DrydockRepositoryOperationStatusView.php:209:        'style' => 'display: none',
src/applications/files/view/PhabricatorGlobalUploadTargetView.php:102:        'style' => 'display: none;',
src/applications/harbormaster/controller/HarbormasterBuildViewController.php:391:            'style' => 'display: none',
src/applications/ponder/controller/PonderQuestionViewController.php:75:        'style' => 'display: none;',
src/applications/ponder/view/PonderAnswerView.php:134:        'style' => 'display: none;',
src/applications/ponder/view/PonderFooterView.php:63:        'style' => 'display: none;',
src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php:431:        'style' => 'display: none',
src/view/layout/AphrontListFilterView.php:70:          'style' => 'display: none;',
src/view/layout/AphrontListFilterView.php:97:            'style' => 'display: none;',
src/view/page/menu/PhabricatorMainMenuView.php:442:          'style' => 'display: none;',
src/view/page/menu/PhabricatorMainMenuView.php:523:          'style' => 'display: none;',
src/view/page/menu/PhabricatorMainMenuView.php:608:            'style' => 'display: none;',
src/view/page/menu/PhabricatorMainMenuView.php:672:            'style' => 'display: none;',
src/view/phui/PHUITimelineEventView.php:396:        'style' => (nonempty($image_uri)) ? '' : 'display: none;',
src/view/phui/PHUIInfoView.php:118:      'style' => $this->isHidden ? 'display: none;' : null,
src/view/phui/PHUIObjectBoxView.php:246:            'style' => $show_style,
src/view/phui/PHUITabGroupView.php:100:          'style' => $style,

These styles hide elements to build the right initial state for Javascript interactions. They can be replaced with a class, but their corresponding Javascript interactions must be tested carefully and display: none on a class is less specific than display: none on the element itself.

Images
src/applications/auth/view/PhabricatorAuthAccountView.php:103:        'style' => 'background-image: url('.$image_uri.');',
src/applications/conpherence/view/ConpherenceDurableColumnView.php:238:              'style' => 'background-image: url('.$image.')',
src/applications/conpherence/view/ConpherenceMenuItemView.php:82:          'style' => 'background-image: url('.$this->imageURI.');',
src/applications/conpherence/view/ConpherenceTransactionView.php:205:            'style' => 'background-image: url('.$image_uri.');',
src/applications/phame/view/PhameBlogListView.php:29:          'style' => 'background-image: url('.$image_uri.');',
src/applications/phame/view/PhameDescriptionView.php:46:        'style' => 'background-image: url('.$this->image.');',
src/applications/phame/view/PhameDraftListView.php:37:          'style' => 'background-image: url('.$image_uri.');',
src/applications/slowvote/view/SlowvoteEmbedView.php:281:          'style' => 'background-image: url('.$handle->getImageURI().')',
src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php:232:        'style' => 'background-image: url('.$image_uri.')',
src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php:415:        'style' => implode(' ', $style),
src/view/phui/PHUIHeadThingView.php:59:        'style' => 'background-image: url('.$this->image.');',
src/view/phui/PHUIHeaderView.php:222:          'style' => 'background-image: url('.$this->image.')',
src/view/phui/PHUIObjectItemView.php:606:          'style' => 'background-image: url('.$this->getImageURI().')',
src/view/phui/PHUIObjectItemView.php:827:      'style' => 'background-image: url('.$handle->getImageURI().')',
src/view/phui/PHUIHovercardView.php:160:            'style' => 'background-image: url('.$handle->getImageURI().');',
src/view/phui/PHUITimelineEventView.php:406:          'style' => 'background-image: url('.$image_uri.')',
src/view/page/menu/PhabricatorMainMenuView.php:316:        'style' => implode(' ', $logo_style),
src/view/phui/PHUIIconView.php:122:      'style' => $style,

These styles are used to add background images to elements. They can conceivably be replaced with <img /> tags but this is often significantly more complex.

Not Reachable
webroot/rsrc/externals/javelin/ext/reactor/dom/RDOM.js:376:        if (key === 'style') {
webroot/rsrc/externals/javelin/ext/reactor/dom/RDOM.js:392:          if (keys[ii] === 'style') {

These calls are in unreachable code.

Not a Style Attribute
src/aphront/response/AphrontStandaloneHTMLResponse.php:54:        'style',

False positives from grep being a crude tool.

Debugging
src/aphront/response/AphrontWebpageResponse.php:38:          'style' => implode(' ', $style),

This style is used to style a debugging interface. The goal of using a "style" attribute here is to render the interface in a readable way even if CSS does not load (for example, because the message is related to a bug in the CSS pipeline).

Progress Bars
src/applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php:153:          'style' =>
src/view/widget/bars/AphrontGlyphBarView.php:87:                'style' => "width: {$percentage}%;",
src/view/widget/bars/AphrontProgressBarView.php:49:            array('style' => "width: {$width}px;"),
src/applications/slowvote/view/SlowvoteEmbedView.php:217:        'style' => sprintf(
src/view/control/AphrontTableView.php:242:            'style' => $style,
src/view/phui/PHUISegmentBarSegmentView.php:73:      'style' => "left: {$left}; width: {$width};",

These styles are used to render a progress bar that works properly without Javascript (or something similar).

Depth
src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php:134:          'style' => $style,

These styles are used to render a hierarchical list of elements that displays correctly without Javascript.

They can be rewritten to use nested lists but this is probably a significant undertaking.

Dynamic Dimensions
src/applications/paste/view/PasteEmbedView.php:54:      $body_attributes['style'] = 'max-height: '.$this->lines * (1.15).'em;';
src/applications/pholio/view/PholioMockThumbGridView.php:134:          'style' => 'top: '.floor((100 - $y) / 2).'px',
src/view/phui/PHUIImageMaskView.php:103:          'style' => implode(' ', $mstyles),
src/view/phui/PHUIImageMaskView.php:112:        'style' => implode(' ', $styles),
src/view/layout/AphrontSideNavFilterView.php:250:          'style' => $width_drag_style,
src/view/layout/AphrontSideNavFilterView.php:271:          'style' => $width_panel_style,
src/view/layout/AphrontSideNavFilterView.php:326:            'style' => $width_margin_style,

These styles are used to position elements based on content information without Javascript.

Dynamic Gradient
src/applications/diffusion/controller/DiffusionBrowseController.php:1238:          'style' => $style,
src/applications/diffusion/controller/DiffusionBrowseController.php:1247:          'style' => $style,
src/applications/diffusion/controller/DiffusionBrowseController.php:1833:          'style' => $style,

These styles are used to render color gradients which fade evenly from one color to another, used to indicate recency (dark = old, bright = new).

Meta Style Data
src/applications/uiexample/examples/PHUIColorPalletteExample.php:69:          'style' => 'background-color: #'.$color.';',
src/applications/uiexample/examples/PHUIColorPalletteExample.php:80:          'style' => 'background-color: #'.$color.';',
src/applications/uiexample/examples/PHUIColorPalletteExample.php:94:          'style' => 'background-color: #'.$color.';',
src/applications/uiexample/examples/PhabricatorFilesComposeAvatarExample.php:62:          'style' => implode(' ', $styles),

These styles are used to show developer metadata about dynamic style information.

Unused
src/applications/diffusion/controller/DiffusionRepositoryProfilePictureController.php:147:      if (isset($spec['style'])) {
src/applications/diffusion/controller/DiffusionRepositoryProfilePictureController.php:148:        $style = $spec['style'];
src/applications/people/controller/PhabricatorPeopleProfilePictureController.php:187:      if (isset($spec['style'])) {
src/applications/people/controller/PhabricatorPeopleProfilePictureController.php:188:        $style = $spec['style'];
src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php:70:          'style' => '',

These styles are unused and can simply be removed.

Dead Soon
src/applications/harbormaster/view/ShellLogView.php:68:          'style' => 'background-color: #fff;',
src/applications/harbormaster/view/ShellLogView.php:98:        'style' => 'background-color: black; color: white;',
src/applications/harbormaster/view/ShellLogView.php:104:          'style' => 'background-color: black',

These calls are fated for destruction soon anyway.

Other
src/view/AphrontTagView.php:131:      'style' => $this->style,
src/view/form/control/AphrontFormControl.php:235:        'style' => $style,
src/view/form/control/AphrontFormTextAreaControl.php:90:        'style'       => $this->getControlStyle(),
src/view/phui/PHUICrumbsView.php:106:            'style' => $action->getStyle(),
src/view/layout/PhabricatorActionView.php:367:        'style' => implode(' ', $style),

These calls generally take arbitrary style information from callers via a setStyle(...) call or similar. However, there are very few setStyle() / setControlStyle() sorts of calls in the codebase so these aren't necessarily hiding a huge amount of compleixty.