Skip to content
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.

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

SCEditor incorrectly formats and interprets [list] BBCode #3844

Closed
Sesquipedalian opened this issue Jan 15, 2017 · 13 comments
Closed

SCEditor incorrectly formats and interprets [list] BBCode #3844

Sesquipedalian opened this issue Jan 15, 2017 · 13 comments
Labels
Milestone

Comments

@Sesquipedalian
Copy link
Member

Similarly to #3843, inserting a bullet list into a post in WYSIWYG mode doesn't work properly.

  1. Insert a bullet list. Initially, all looks well.
    screen shot 2017-01-15 at 5 04 32 am
  2. Either get a preview or just toggle the editor into source mode. You will see that instead of converting your <ul> into a simple [list] BBCode, it will convert it to [list type=none].
    screen shot 2017-01-15 at 5 08 42 am
    screen shot 2017-01-15 at 5 09 16 am

As with #3843, the problem appears to have been introduced in #3714, and disappears if one reverts those changes to jquery.sceditor.bbcode.min.js.

@Oldiesmann Oldiesmann added this to the Beta 3 milestone Jan 19, 2017
@ThomasFok
Copy link
Contributor

I found the cause of the problem.

Themes/default/scripts/jquery.sceditor.smf.js line 277-282

format: function (element, content) {
	if ($(element[0]).css('list-style-type') == 'disc')
		return '[list]' + content + '[/list]';
	else
		return '[list type=' + $(element[0]).css('list-style-type') + ']' + content + '[/list]';
}

The css() function should return 'disc' but it returns 'none'. Since the default value of list-style-type is 'disc', I have no idea why the function return 'none'.

@Sesquipedalian
Copy link
Member Author

That's helpful, @ThomasFok. Perhaps it is because index.css defines list-style-type: none as the default for lists.

@Sesquipedalian
Copy link
Member Author

If so, maybe a fix would be as simple as adding a scoped definition in the CSS to make list-style-type: disc the default inside the editor.

@Oldiesmann
Copy link
Contributor

Here's a diff of the two versions of jquery.sceditor.bbcode.min.js (un-minified): https://www.diffchecker.com/TCE3rbLL. I'm not sure what changed exactly to make it do that. I compared the two versions of jquery.sceditor.smf.js and nothing really changed there. It does look like we can get around the issue with CSS, but I'm curious as to why that wasn't necessary before.

@ThomasFok
Copy link
Contributor

If list-style in index.css is changed to disc, it works. But I cannot find a proper scoped CSS rule that can fix the problem. .sceditor-container ul{} does not work.

@ThomasFok
Copy link
Contributor

I found a workaround for the problem. When the [list] bbcode is converted to html, a style attribute which stores the list-style-type is added to the ul tag. So, we may get the list-style-type from there.

I have the code already and it seems to work. I can post the code if nobody can find a better fix.

@Sesquipedalian
Copy link
Member Author

Instead of index.css, try adding the rule to jquery.sceditor.css. I think that might work better.

@Sesquipedalian
Copy link
Member Author

Sorry, I meant jquery.sceditor.default.css. That's the one that styles inside the editor's iframe.

@ThomasFok
Copy link
Contributor

Adding ul{list-style-type:disc;} to jquery.sceditor.default.css does not work.

Then, I tried adding this to jquery.sceditor.default.css. The list bullets did change to circles.

ul, ol {
	list-style-type: circle;
}

image

Switching to source mode, it still showed type=none.

image

I think the root of the problem is inside jquery.sceditor.bbcode.min.js.

@sbulen
Copy link
Contributor

sbulen commented Jan 25, 2017

There is some pre-processing of list-style-type in sub.php & subs-editor.php. Might be worth a look.

@Sesquipedalian
Copy link
Member Author

Sesquipedalian commented Jan 25, 2017

Oh hey, I just solved it. :) #3868

@Sesquipedalian
Copy link
Member Author

Turns out it was just a matter of adding a test for when the style attribute was undefined. That way we don't have to worry about inherited values, but only whatever has been explicitly set on the element itself.

@ThomasFok
Copy link
Contributor

@Sesquipedalian, it is a good fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants