Skip to content

fix #418 Use of inline styles doesn't play nice with Content Security Policy #1976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

fix #418 Use of inline styles doesn't play nice with Content Security Policy #1976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
170ebba
* updating styles directly on HTMLELement style description rather th…
shaurya-sisodia Jun 13, 2022
168b27a
review fix
shaurya-sisodia Jun 15, 2022
0d01844
indent fix
shaurya-sisodia Jun 15, 2022
8545cc1
incorporating review fix and fix bugs breaking the demo
shaurya-sisodia Jun 19, 2022
b89bee7
Merge branch 'master' into fix-#418-Use-of-inline-styles-doesn't-play…
shaurya-sisodia Jun 20, 2022
031b3ba
Some additional clean up
shaurya-sisodia Jun 20, 2022
a2e3318
* using adoptedStyleSheets to create dynamic stylesheet and replaceSy…
shaurya-sisodia Jul 21, 2022
e3a032e
Merge branch 'master' into fix-#418-Use-of-inline-styles-doesn't-play…
shaurya-sisodia Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 1 addition & 58 deletions spec/gridstack-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,20 +246,7 @@ describe('gridstack', function() {
expect(grid.getColumn()).toBe(12);
grid.column(12);
expect(grid.getColumn()).toBe(12);
});
it('should set construct CSS class', function() {
let grid = GridStack.init({column: 1});
expect(grid.el.classList.contains('grid-stack-1')).toBe(true);
grid.column(2);
expect(grid.el.classList.contains('grid-stack-1')).toBe(false);
expect(grid.el.classList.contains('grid-stack-2')).toBe(true);
});
it('should set CSS class', function() {
let grid = GridStack.init();
expect(grid.el.classList.contains('grid-stack')).toBe(true);
grid.column(1);
expect(grid.el.classList.contains('grid-stack-1')).toBe(true);
});
});
it('should SMALL change column number, no relayout', function() {
let options = {
column: 12
Expand Down Expand Up @@ -1362,51 +1349,7 @@ describe('gridstack', function() {
expect(grid.el.classList.contains('grid-stack-rtl')).toBe(false);
});
});

describe('grid.opts.styleInHead', function() {
beforeEach(function() {
document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
});
afterEach(function() {
document.body.removeChild(document.getElementById('gs-cont'));
});
it('should add STYLE to parent node as a default', function() {
var options = {
cellHeight: 80,
verticalMargin: 10,
float: false,
};
var grid = GridStack.init(options);
expect((grid as any)._styles.ownerNode.parentNode.tagName).toBe('DIV'); // any to access private _styles
});
it('should add STYLE to HEAD if styleInHead === true', function() {
var options = {
cellHeight: 80,
verticalMargin: 10,
float: false,
styleInHead: true
};
var grid = GridStack.init(options);
expect((grid as any)._styles.ownerNode.parentNode.tagName).toBe('HEAD'); // any to access private _styles
});
});

describe('grid.opts.styleInHead', function() {
beforeEach(function() {
document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
});
afterEach(function() {
document.body.removeChild(document.getElementById('gs-cont'));
});
it('should add STYLE to parent node as a default', function() {
var grid = GridStack.init();
expect((grid as any)._styles.ownerNode.parentNode.tagName).toBe('DIV');
});
it('should add STYLE to HEAD if styleInHead === true', function() {
var grid = GridStack.init({styleInHead: true});
expect((grid as any)._styles.ownerNode.parentNode.tagName).toBe('HEAD');
});
});

describe('grid.enableMove', function() {
beforeEach(function() {
Expand Down
16 changes: 0 additions & 16 deletions spec/utils-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,6 @@ describe('gridstack utils', function() {
});
});

describe('test createStylesheet/removeStylesheet', function() {

it('should create/remove style DOM', function() {
let _id = 'test-123';
Utils.createStylesheet(_id);

let style = document.querySelector('STYLE[gs-style-id=' + _id + ']');
expect(style).not.toBe(null);
// expect(style.prop('tagName')).toEqual('STYLE');

Utils.removeStylesheet(_id)
style = document.querySelector('STYLE[gs-style-id=' + _id + ']');
expect(style).toBe(null);
});

});

describe('test parseHeight', function() {

Expand Down
3 changes: 3 additions & 0 deletions src/gridstack-dd.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ GridStack.prototype._setupAcceptWidget = function(this: GridStack): GridStack {
} else {
this.engine.removeNode(node);
}
if(!Utils.isConstructableStyleSheetSupported()) {
this._updateElementChildrenStyling(node.el);
}
});

return false; // prevent parent from receiving msg (which may be grid as well)
Expand Down
88 changes: 85 additions & 3 deletions src/gridstack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ export class GridStack {
public _gsEventHandler = {};
/** @internal */
protected _styles: GridCSSStyleSheet;
/** @internal max row index of the gridstack*/
private _maxRowIndex: number;
/** @internal flag to keep cells square during resize */
protected _isAutoCellHeight: boolean;
/** @internal track event binding to window resize so we can remove */
Expand Down Expand Up @@ -476,6 +478,10 @@ export class GridStack {

this._triggerAddEvent();
this._triggerChangeEvent();
if(!Utils.isConstructableStyleSheetSupported()) {
Utils.updatePositionStyleOnWidget(el, this.opts.cellHeight as number, this.opts.cellHeightUnit);
this._updateElementChildrenStyling(el);
}

return el;
}
Expand Down Expand Up @@ -1180,16 +1186,18 @@ export class GridStack {
protected _removeStylesheet(): GridStack {

if (this._styles) {
Utils.removeStylesheet(this._styles._id);
Utils.removeStylesheet(this._styles);
delete this._styles;
}
return this;
}

/** @internal updated/create the CSS styles for row based layout and initial margin setting */
/** @internal updated/create the CSS styles for row based layout and initial margin setting
* in case no constructable stylesheet support updated/create the CSS styles on elements for row based layout and initial margin setting */
protected _updateStyles(forceUpdate = false, maxH?: number): GridStack {
// call to delete existing one if we change cellHeight / margin
if (forceUpdate) {
this._maxRowIndex = undefined;
this._removeStylesheet();
}

Expand All @@ -1204,12 +1212,56 @@ export class GridStack {
let cellHeightUnit = this.opts.cellHeightUnit;
let prefix = `.${this.opts._styleSheetClass} > .${this.opts.itemClass}`;

if(Utils.isConstructableStyleSheetSupported()) {
return this._updateStyleRules(cellHeight, cellHeightUnit, prefix, maxH);
} else {
return this._updatedStylesForElements(cellHeight, cellHeightUnit, prefix, maxH);
}
}

/** @internal update Styling */
protected _updateElementChildrenStyling(el?: HTMLElement, prefix?: string): void {
let top: string = this.opts.marginTop + this.opts.marginUnit;
let bottom: string = this.opts.marginBottom + this.opts.marginUnit;
let right: string = this.opts.marginRight + this.opts.marginUnit;
let left: string = this.opts.marginLeft + this.opts.marginUnit;
let content: string;
let placeholder = `.${this.opts._styleSheetClass} > .grid-stack-placeholder > .placeholder-content`;
if (el!==undefined) {
content = `.grid-stack-item-content`
// content margins
Utils.updateStyleOnElements([el.querySelector(content) as HTMLElement], {top, right, bottom, left});
Utils.updateStyleOnElements(placeholder, {top, right, bottom, left});
// resize handles offset (to match margin)
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-ne`) as HTMLElement], {right});
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-e`) as HTMLElement], {right});
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-se`) as HTMLElement], {right, bottom});
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-nw`) as HTMLElement], {left});
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-w`) as HTMLElement], {left});
Utils.updateStyleOnElements([el.querySelector(`.ui-resizable-sw`) as HTMLElement], {left, bottom});
} else if (prefix!= undefined) {
content = `${prefix} > .grid-stack-item-content`;
// content margins
Utils.updateStyleOnElements(content, {top, right, bottom, left});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

every single new element will also need those attributes, so how do you re-use this code since it isn't a css style anymore ?

Utils.updateStyleOnElements(placeholder, {top, right, bottom, left});
// resize handles offset (to match margin)
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-ne`, {right});
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-e`, {right});
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-se`, {right, bottom});
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-nw`, {left});
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-w`, {left});
Utils.updateStyleOnElements(`${prefix} > .ui-resizable-sw`, {left, bottom});
}
}

/**@internal */
protected _updateStyleRules(cellHeight:number, cellHeightUnit:string, prefix:string, maxH?: number): GridStack {
// create one as needed
if (!this._styles) {
let id = 'gridstack-style-' + (Math.random() * 100000).toFixed();
// insert style to parent (instead of 'head' by default) to support WebComponent
let styleLocation = this.opts.styleInHead ? undefined : this.el.parentNode as HTMLElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styleInHead needs to be removed everywhere (doc and code) as it no longer applies

this._styles = Utils.createStylesheet(id, styleLocation);
this._styles = Utils.createStylesheet(styleLocation);
if (!this._styles) return this;
this._styles._id = id;
this._styles._max = 0;
Expand Down Expand Up @@ -1250,6 +1302,32 @@ export class GridStack {
return this;
}

/**@internal updates styles for dirty nodes */
protected _updatedStylesForElements(cellHeight:number, cellHeightUnit:string, prefix:string, maxH?: number): GridStack {
// create one as needed
if (!this._maxRowIndex) {
this._maxRowIndex = 0;

// these are done once only
Utils.updateStyleOnElements(prefix, {'min-height': cellHeight+cellHeightUnit});
this._updateElementChildrenStyling(undefined, prefix);
}

// apply position styling on all dirty node's elements
this.getGridItems().forEach((w: GridItemHTMLElement) => {
if (w.gridstackNode._dirty && !w.classList.contains("ui-resizable-resizing")) {
Utils.updatePositionStyleOnWidget(w, this.opts.cellHeight as number, this.opts.cellHeightUnit);
}
});

// now update the height specific fields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment doesn't apply to code below, which looks like what you should do instead of the this._maxRowIndex = 0; you are doing above...

maxH = maxH || this._maxRowIndex;
if (maxH > this._maxRowIndex) {
this._maxRowIndex = maxH;
}
return this;
}

/** @internal */
protected _updateContainerHeight(): GridStack {
if (!this.engine || this.engine.batchMode) return this;
Expand Down Expand Up @@ -1301,6 +1379,10 @@ export class GridStack {
if (n.y !== undefined && n.y !== null) { el.setAttribute('gs-y', String(n.y)); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the all reason for writing gs-x, gs-y, gs-w, gs-h IS to use global CSS styling to position them, but if we use attributes directly instead I don't see a reason to have any of this anymore... which is a much bigger change.
Also the big implication is that external framework that write to these values for initial placement would have to be changed... humm. not sure I'm ready for such a drastic change. will have to think more about it...

if (n.w) { el.setAttribute('gs-w', String(n.w)); }
if (n.h) { el.setAttribute('gs-h', String(n.h)); }
if(!Utils.isConstructableStyleSheetSupported()) {
Utils.updatePositionStyleOnWidget(el, this.opts.cellHeight as number, this.opts.cellHeightUnit);
this._updateElementChildrenStyling(el);
}
return this;
}

Expand Down
Loading