Item2254: Fix to Item1798 introduces cursor problems for Moz browsers

Priority: CurrentState: AppliesTo: Component: WaitingFor:
Normal Closed Extension TinyMCEPlugin, WysiwygPlugin Main.PaulHarvey, Main.MichaelTempest
Item1798 introduces forced_root_blocks: false to fix a problem where extraneous <p> tags are added.

However this introduces strange "cursor jumping" behaviour, especially when working with bullets (easy to reproduce: see TestTopicWithDefaultTMCE, and the "fixed" TINYMCEPLUGIN_INIT at TestTopicWithForceRootBlocks).

The above test topics demonstrate easy to reproduce cursor jumping problems when trying to add newlines after a bullet list.

I have users telling me it sometimes happens with Heading lines as well, although I can't reproduce that.

Setting forced_root_blocks: false also inhibits a work-around the moxiecode guys had for the infamous cursor-trapped-in-tables where you couldn't get the cursor beyond (or before) a table at the end (or beginning) of a document in non-IE browsers.

I will try to raise a bug upstream; in the meantime, we should decide if there is anything we can do to alleviate this problem for now.

-- PaulHarvey - 15 Oct 2009

Moxiecode FAQ explaining new behaviour.

Threads of interest: p tags around hr,table,div etc., cursor trapped in tables work-around introduces <p/> elements, If you have troubles placing the cursor after the last table.. ([a plugin created before the official fix) (download)

-- PaulHarvey - 15 Oct 2009

This is limited to gecko browsers.

Problem replicated on these browsers:
  • Iceweasel/Firefox 3.0.12 (Linux)
  • Firefox 3.5.3 (Windows XP SP3)
  • Epiphany 2.22.3 (Linux)

Unable to replicate on:
  • Opera 10.00 build 4585 (Linux)
  • Opera 10.00 build 1750 (Windows XP SP3)
  • Google Chrome 3.0.195.27 (Windows XP SP3)
  • Safari 4.0.3 (531.9.1) (Windows XP SP3)
  • Internet Explorer 6.0.2900.5512
  • Internet Explorer 8.00.6001

-- PaulHarvey - 15 Oct 2009

bug reported upstream

-- PaulHarvey - 15 Oct 2009

Added comment on sf.net where I found a few lines of code that can be commented out to hide this problem.

-- PaulHarvey - 15 Oct 2009

I've been poring over ForceBlocks.js (specifically insertPara where the problem is).

There's code there that apparently affects Gecko browsers (though there's no specific sniffing) which works around the problem that it seems impossible to get the caret position in situations where (as far as I can tell) non-block elements like <br/> end up inside <body>. Sounds like a gecko bug to me, but I don't know much... Compare to chrome, which the DOM browser seems to show insertion of DIVs.

Commenting out the code that forces the cursor to jump to the top of the document results in a series of <br/> newlines after an li or ol list, as opposed to <p> paras normally.

Not sure where to go from here. As a few important cursor bugs seem to be fixed with forced_root_blocks not disabled, I am feeling more and more that perhaps we should look at how we can perhaps tune WysiwygPlugin to cope with TMCE's default init parameters.

-- PaulHarvey - 16 Oct 2009

I agree - the right way to go is to make WysiwygPlugin produce (and consume) HTML more to TMCE's liking. I am nervous of a change like that, but I think it is needed.

-- MichaelTempest - 16 Oct 2009

Actually, this is a very sticky problem. I've been trying to think of how WysiwygPlugin could handle the tables issue without forced_root_blocks: false and it's not coming to me...

I have a regrettably adventurous patch which I doubt upstream would merge, that could fix the bullets situation with forced_root_blocks: false, but we'd still have to put up with the cursor getting trapped in tables problem I suppose.

-- PaulHarvey - 16 Oct 2009

Downgraded to Normal, this bug has been with us since 1.0.6.

-- PaulHarvey - 18 Oct 2009

Have been trying to make my work-around for this work as a plugin. It's not working out; overriding an internal TMCE function is proving very difficult. So this will have to be a patch to classes/ForceBlocks.js.

-- PaulHarvey - 19 Oct 2009

Added some deeper analysis to Item8274

-- PaulHarvey - 20 Oct 2009

Don't bother with the release files below, they contain an error :(

-- PaulHarvey - 20 Oct 2009

Actually, forced_root_blocks: false also prevents Opera 10 from playing nice as well.

-- PaulHarvey - 21 Oct 2009

Right. The proper way is to just remove forced_root_blocks : false.

And then the fix for the tables (and other) problem in Item1798 is to just ensure that TinyMCE receives all the $content inside a <div>. Bang, no more non-block elments without a block container to confuse web browser node selections smile

Just to fix the unit tests...

-- PaulHarvey - 21 Oct 2009

I have been running this seemingly without being any worse off. The list item bugs are gone, Opera behaves properly and everything seems happy.

However it seems TMCE's fixes for the cursor being unable to get beyond a table that's positioned at the beginning or end of a document only works if the table is a top-level element.

So I think if we want that bug fixed, we have to only put divs around table + text that immediately follows.

-- PaulHarvey - 21 Oct 2009

I committed Foswikirev:5347. TML2HTML wraps the output in <div></div>. I adjusted the unit tests so that they will pass with this arrangement.

After some review. hopefully CrawfordCurrie and MichaelTempest, I would like to a make release and ask people on foswiki-discuss to test it.

-- PaulHarvey - 22 Oct 2009

I have to spend some time on other things. We won't have a proper fix for this for 1.0.8.

-- PaulHarvey - 22 Oct 2009

I can make this all work, including TMCE's fixes for the cursor being unable to get beyond a table that is positioned at the beginning or end of a document.
  1. Set forced_root_block to its default (i.e. remove the setting from TINYMCEPLUGIN_INIT)
  2. Wrap the output of TML2HTML in a <div>
  3. Open the fullscreen view after editing the topic. (It still works after closing the fullscreen view.)

PaulHarvey proposed 1 and 2, and I support that change. I believe that 3 is evidence of a bug in TMCE or in TinyMCEPlugin - this action also fixes other problems e.g. Item5955.

(To move before a table at the beginning of a document, I had to use Ctrl-left-arrow. Similarly, I had to use Ctrl-right-arrow to move beyond a table at the end of a document. I discovered these by accident. Are they documented somewhere?)

-- MichaelTempest - 21 Nov 2009

I suppose ctrl+arrow is probably a browser thing, rather than TMCE.

It seems gecko's node selection stays internally consistent for the browser GUI itself; in that - even though the Javascript DOM starts reporting nonsense about which node is currently selected with the cursor position - when running with TMCE's cursor-reset-to-top-of-page code disabled, we can still work in the rich text editor just fine (but things like mceInsertContent command won't insert at the cursor position when it's invalid).

So, what am I saying...
  • Perhaps we can teach WysiwygPlugin to remove the <p> around the table? If the user really wanted a newline after the table, they could insert an empty <p></p> after it.
  • Perhaps we can teach WysiwygPlugin to wrap tables in their own <div>s.
  • Speaking of <p>s and tables, should we teach WysiwygPlugin to eat the <p>s that end up in table cells, as per Item5221...

-- PaulHarvey - 22 Nov 2009

Item2447 is also caused by this. Michael, added a comment about your proposal (3) in Item5955.

Obviously we can't commit this patch yet, apart from needing more testing:
  • end-of-div and end-of-para uses the same logic. Refactor those four lines which are copy-pasted everywhere into a common method.
  • We don't want divs around our tables unless there really is a line of text that immediately follows it.
  • Needs unit tests.

It does however, allow me to run without forced_root_blocks: false, and it does mean we haven't cheated by wrapping everything in a div, so we are much closer to Moxiecode's recommended usage of TinyMCE which should hopefully mean less bugs smile

-- PaulHarvey - 05 Dec 2009

Actually, the patch I have recently attached is bogus - it should only merge following text into a common block element if there's only one newline separating the table and the following text. Instead it forces all following text ... but anyway, that's why it's proof of concept.

MichaelTempest: We could feasibly ship Foswikirev:5347 as you say as a temporary solution. Have you given it much testing?

I have created Item2471 to implement what I think is the "better" solution...

-- PaulHarvey - 07 Dec 2009

Actually, as of TinyMCE 3.3.3 - we don't get cursor jumping any more when finishing bullet lists in Firefox smile

-- PaulHarvey - 20 Apr 2010

WysiwygPlugin wraps tables in divs if there are macros before or after the table. TinyMCEPlugin no longer sets force_root_blocks: false.

I have done some testing. More is needed.

-- MichaelTempest - 15 May 2010

Michael, have been using this for two weeks on our production site, so far so good! Overall, trunk WysiwygPlugin+TMCE seems to be an improvement over 1.0.9. Great work

-- PaulHarvey - 05 Jun 2010

smile I plan no further changes on this task, so I am setting it to "waiting for release"

-- MichaelTempest - 07 Jun 2010
 
Topic attachments
I Attachment Action Size Date Who Comment
TML2HTML.pm.diffdiff TML2HTML.pm.diff manage 2.8 K 06 Dec 2009 - 04:57 PaulHarvey Proof of concept div-around-tables+follwing-paras fix
TinyMCEPlugin.md5md5 TinyMCEPlugin.md5 manage 0.2 K 22 Oct 2009 - 14:37 PaulHarvey 3.2.7 with Foswikirev:5347
TinyMCEPlugin.sha1sha1 TinyMCEPlugin.sha1 manage 0.2 K 22 Oct 2009 - 14:37 PaulHarvey 3.2.7 with Foswikirev:5347
TinyMCEPlugin.tgztgz TinyMCEPlugin.tgz manage 1082.9 K 22 Oct 2009 - 14:38 PaulHarvey 3.2.7 with Foswikirev:5347
TinyMCEPlugin.zipzip TinyMCEPlugin.zip manage 1295.9 K 22 Oct 2009 - 14:38 PaulHarvey 3.2.7 with Foswikirev:5347
TinyMCEPlugin_installerEXT TinyMCEPlugin_installer manage 38.3 K 22 Oct 2009 - 14:39 PaulHarvey 3.2.7 with Foswikirev:5347
WysiwygPlugin.md5md5 WysiwygPlugin.md5 manage 0.2 K 22 Oct 2009 - 15:06 PaulHarvey Foswikirev:5347
WysiwygPlugin.sha1sha1 WysiwygPlugin.sha1 manage 0.2 K 22 Oct 2009 - 15:06 PaulHarvey Foswikirev:5347
WysiwygPlugin.tgztgz WysiwygPlugin.tgz manage 49.3 K 22 Oct 2009 - 15:06 PaulHarvey Foswikirev:5347
WysiwygPlugin.zipzip WysiwygPlugin.zip manage 57.3 K 22 Oct 2009 - 15:09 PaulHarvey Foswikirev:5347
WysiwygPlugin_installerEXT WysiwygPlugin_installer manage 5.5 K 22 Oct 2009 - 15:11 PaulHarvey Foswikirev:5347
test_htmlEXT test_html manage 0.5 K 15 Oct 2009 - 05:46 PaulHarvey Test case to submit to upstream
tinyMCE_3.2.7.diffdiff tinyMCE_3.2.7.diff manage 2.9 K 20 Oct 2009 - 05:46 PaulHarvey  
tinyMCE_trunk.diffdiff tinyMCE_trunk.diff manage 2.8 K 20 Oct 2009 - 03:35 PaulHarvey Patch to trunk rev1250 of tinyMCE
Topic revision: r45 - 04 Oct 2010, KennethLavrsen
 
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement. Creative Commons License