Opened 6 years ago

Closed 6 years ago

#10136 closed Bug (fixed)

Error when running Stylesheet Parser sample

Reported by: Olek Nowodziński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.1 RC
Component: Core : Styles Version: 4.0.2
Keywords: Cc:

Description

  1. http://ckeditor4.t/ckeditor/samples/plugins/stylesheetparser/stylesheetparser.html
  2. Uncaught TypeError: Cannot read property '$' of undefined
  3. Error is in plugin.js:101

Change History (10)

comment:1 Changed 6 years ago by Jakub Ś

Status: newconfirmed

To reproduce:

  1. Get the latest major version
  2. Reload cache in browser and load stylesheetparser sample.

comment:2 Changed 6 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:3 Changed 6 years ago by Piotrek Koszuliński

Status: assignedreview

Add this to the stylesheet parser sample:

on: {
	setData: function( evt ) {
		console.log( 'setData', evt.editor.document.$.styleSheets.length );
	},

	contentDom: function( evt ) {
		console.log( 'contentDom', evt.editor.document.$.styleSheets.length );
	},

	instanceReady: function( evt ) {
		console.log( 'instanceReady', evt.editor.document.$.styleSheets.length );
	}
}

Result:

setData 1
contentDom 3
instanceReady 3

On setData stylesheets are not ready yet. To parse them before data are loaded we would have to reinvent bootstrap logic of the wysiwygarea editable.

Thus, my proposal is that stylesheet parser should parse only files specified in config.contentsCss or to avoid duplication with styles defined in styles.js special setting - config.stylesheetParser_files.

We'll be able to preload these files separately, before initializing editable. This will also fix case when startupMode != 'wysiwyg'.

However, for now we need a quick hack that would make stylesheet parser working without integration with ACF. Pushed t/10136 with patch that does the job. It just postpones the moment when stylescombo creates combo's items. Thanks to that stylescombo waits for stylesheet parser that has to wait until editor.document is ready (contentDom). The only thing that this patch breaks is that styles combo won't be hidden when there are no styles at all.

comment:4 Changed 6 years ago by Olek Nowodziński

Status: reviewreview_failed

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

Additionally, shoudn't we have "once" as well in the "contentDom" listener in stylesheetparser/plugin.js?

comment:6 in reply to:  4 Changed 6 years ago by Piotrek Koszuliński

Status: review_failedreview

Replying to a.nowodzinski:

The following test fails on t/10136: http://ckeditor4.t/dt/plugins/stylescombo/stylescombo.html?name=test-external-styles-are-registerd-in-ACF

In case when stylesheetparser isn't loaded we can't wait until contentDom, because styles rules have to added to ACF before first setData. Fixed.

Additionally, shoudn't we have "once" as well in the "contentDom" listener in stylesheetparser/plugin.js?

Yep. Fixed.

comment:7 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

The proposed fix creates a implicit dependency between those plugins... if one of them change their code, it will break the other one... the kind of think we should never have.

If no other clear solution is available, we may reconsider this, but we should try a different approach first.

comment:8 Changed 6 years ago by Piotrek Koszuliński

Status: review_failedreview

Pushed dev and tests branches.

I introduced new event - editor#stylesSet. Stylesheetparser cancels it and waits for contentDom to fire it again, this time with all styles. Styles combo listens on stylesSet too and therefore is able to initialize styles asap.

Last edited 6 years ago by Piotrek Koszuliński (previous) (diff)

comment:9 Changed 6 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:10 Changed 6 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on major with git:8abbc10 on dev and f00bdbd on tests.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy