Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Webkit: inserting DIV breaks DOM #352

Merged
merged 4 commits into from
Jun 18, 2012
Merged

Webkit: inserting DIV breaks DOM #352

merged 4 commits into from
Jun 18, 2012

Conversation

jkrcma
Copy link

@jkrcma jkrcma commented May 16, 2012

refs to #319, #314

Hi,

I was given some time by a company and here is the result. Still I'm not sure if it is possible to profit from this bugfix unless WYMeditor.DIV type is allowed in WYMeditor.editor.container() method, which I need to override.

@winhamwr
Copy link
Member

Hello,

Thanks so much for the pull request! I really hope we can get this DIV thing fixed for you.

I'm taking a look at this right now, but a couple of things I'm noticing right away:

Feel free to drop by #wymeditor on freenode if you want to discuss this.

Thanks again
-Wes

@jkrcma
Copy link
Author

jkrcma commented May 17, 2012

I should have mentioned this commit was just a proposal in order to open a discussion on a particular topic. I'm sorry for flaws, I'll fix them as soon as I'm able to.

To the container() method mentioned before, it's not related. This fix should be just a Webkit hack usable in any JS editor.

@winhamwr
Copy link
Member

Wonderful!

I'd like for this test to pass in all of the browsers if possible (instead of just safari/webkit). Even though the bug is only present there right now, it would be nice as a regression test if behavior changes in those browsers in the future.

I have a flight this evening and my plan is to get this reviewed and pulled in from 30k feet.

Thanks again
-Wes

@jkrcma
Copy link
Author

jkrcma commented May 17, 2012

From my and company observations Firefox respects the DOM automatically and correctly. Nice example of its behavior is automatic contextual switching of <dt> and <dd>, although WYM doesn't support it yet. To the IEs, yes, that is the question! :)

I'm able to bother my boss to test the behavior in various versions of IE tomorrow if it helps.

winhamwr added a commit that referenced this pull request Jun 18, 2012
@winhamwr winhamwr merged commit 443b9f8 into wymeditor:master Jun 18, 2012
@winhamwr
Copy link
Member

Merged in. I apologize for taking so long here. I finished up some things this month that should allow me more time for WYMeditor improvements (and faster inclusion of pull requests).

Thanks again for putting this pull request together.
-Wes

@winhamwr
Copy link
Member

Looks like I'm going to have to revert at least the part of the fix that stops divs from being converted to p tags when they get selection. That's required to keep webkit from making everything a div. Allowing a user to keep root divs is going to require a more fundamental change I think. We'll have to make it a configuration option where you can either choose between divs or paragraphs as the base/default container unit.

@jkrcma
Copy link
Author

jkrcma commented Jul 11, 2012

Whoa, I missed that although this is typical use case in our company. Looks like our editors use Paragraph button often. The goal to fix this should be in achieving the existing divs to be kept in the document but paragraphs to be created by default. How complicated could this be?

@winhamwr
Copy link
Member

The fix for this is turning out to be pretty involved. I'm going to revert this merge in master and un-revert it when #360 is ready. For folks who absolutely need DIV support, you can run on 1.0.0b3. I'm hoping to land #360 in 1.0.0b5, but there are some good things since the 1.0.0b3 release and I need to stop holding those up for this change.

winhamwr added a commit that referenced this pull request Feb 15, 2013
Issue #360 will have a fully-baked fix for this.
@winhamwr
Copy link
Member

#360 just landed, so I'd love to have you try this out, if you get a chance.

The goal to fix this should be in achieving the existing divs to be kept in the document but paragraphs to be created by default. How complicated could this be?

This is now the default behavior. If a user creates a new block, it will be a paragraph, but existing divs will only be changed to paragraphs if the user moves their cursor in to them.

lehni pushed a commit to wemakeit/wymeditor that referenced this pull request May 15, 2014
…nd-of-breaking.

Issue wymeditor#360 will have a fully-baked fix for this.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants