Opened 5 years ago

Closed 16 months ago

Last modified 6 months ago

#10415 closed task (fixed)

Make dijit.Editor use contentEditable in Firefox whenever it supports it

Reported by: MarcosPri Owned by: bill
Priority: low Milestone: 1.7.6
Component: Editor Version: 1.4.0b
Keywords: Editor, Firefox Cc: markken@…
Blocked by: Blocking:

Description

Now the editor is using desingmode in all version of Firefox but it supports contentEditable since version 3.

It seems now that contentEditable is going into HTML 5 specification so sounds like a good idea to use it in Firefox from version 3+ and avoid the annoyances of two different implementations for the editor whenever possible.

Attachments (2)

ContentEditable_TabNavFails.patch​ (1.7 KB) - added by jaredj 4 years ago.
This swaps to ContentEditable, but then the A11Y tab nav tests fail. Very odd.
RichText.js.diff​ (1.9 KB) - added by jsonn 16 months ago.
Updated patch against 1.9.1

Download all attachments as: .zip

Change History (21)

comment:1 Changed 5 years ago by bill

  • Resolution set to duplicate
  • Status changed from new to closed

Dup of #6582.

comment:2 Changed 5 years ago by bill

PS: your comment about "avoid the annoyances of two different implementations for the editor" is inaccurate, as editor is currently using an iframe everywhere... there's only one implementation.

Still, a patch to convert the editor to always use a contentEditable <div> would be welcome. Note that dojo no longer supports FF2 or earlier.

We need some kind of solution to support user defined stylesheets (probably automatically rewriting the stylesheet so that each pattern includes the id of the editor's div). Also need to make sure that we don't lose the selected text when using the keyboard to navigate from the content to the toolbar.

comment:3 Changed 5 years ago by MarcosPri

I might be mistaken but I think that even the iframe is used everywhere in ff it's editable using designmode and contentEditable in other browsers.

One of that annoyances is that you can't insert a contentEditable="false" element in the editor to have some not editable parts in it -it will be editable in ff, the whole document is because of designmode- and be cross browser compatible (really a minor annoyance, probably not a very common scenario)

comment:4 Changed 5 years ago by bill

  • Component changed from Dijit to Editor
  • Milestone changed from tbd to 1.5
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Ah sorry, I misunderstood. Yes, I see the RichText._setDisabledAttr() is branching for FF vs. other browsers.

Worth a try to remove that FF specific branch (as you suggested).

comment:5 Changed 5 years ago by youngho

looks like this improvement can be remove #10368 FF variance

comment:6 Changed 5 years ago by jaredj

  • Milestone changed from 1.5 to future

comment:7 Changed 4 years ago by jaredj

Interestingly, I tried this and it seemed to edit fine, but it causes some A11Y tab navigation to fail! Very strange. I'm attaching on what I changed as an example, but need to look into why tab nav fails with it.

Changed 4 years ago by jaredj

This swaps to ContentEditable, but then the A11Y tab nav tests fail. Very odd.

comment:8 Changed 4 years ago by becky

Not sure why using contentEditable changes the tab order. With the patch it takes two tab stops to get from the toolbar into the editor with an insertion caret. When I look at these with inspect32 it shows focus on the same object - the document of the iframe. Not sure why? Need to examine the code more closely.

comment:9 Changed 2 years ago by bill

  • Priority changed from high to low

Changed 16 months ago by jsonn

Updated patch against 1.9.1

comment:10 Changed 16 months ago by jsonn

I'd really like to see this getting resolved since designMode implicitly disables UI elements. This is not done with contentEditable, so that e.g. <select> can be used in the editor.

comment:11 Changed 16 months ago by bill

The good news is that the problem Becky mentioned above seems to have gone away. On the downside, the patch causes new failures in Editor_a11y.html, in the keyboard navigation tests.

Another complication is that there's an existing failure in the Editor_a11y "tabbing" test suite, dealing with detecting when the <iframe> has gotten focus. This patch actually helps to fix that failure; previously we weren't getting any focusin event at all; now we are getting the notification, but it's being ignored due to this code in focus.js, because tag == "body":

// IE reports that nodes like <body> have gotten focus, even though they have tabIndex=-1,
// ignore those events
var tag = evt.target.tagName.toLowerCase();
if(tag == "#document" || tag == "body"){ return; }

comment:12 Changed 16 months ago by bill

  • Milestone changed from future to 1.10
  • Owner set to bill
  • Status changed from reopened to assigned

comment:13 Changed 16 months ago by Bill Keese <bill@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 0854ec27436ba935b09e29c0cfc2960656780070/dijit:

Fixes and refactorings for Editor on Firefox.

Fixes Editor_a11y.html regression test failure and removes firefox specific code branches:

  • make FF use contentEditable (like other browsers) rather than designMode
  • make FF use nested <div> inside of <body> (inside of <iframe>), just like other browsers
  • expand tabbing helper code (both for tabbing/shift-tabbing into and out of the iframe) to run on all browsers, not just IE
  • reduce the number of branches in focus(), so that Chrome and FF use same path

Fixes #10415, #17560.
Refs #11898.

comment:14 Changed 15 months ago by Colin Snover <github.com@…>

In f7e337959c313ccfdeae8e26c629a48e60fa9ed0/dijit:

Fixes and refactorings for Editor on Firefox.

Fixes Editor_a11y.html regression test failure and removes firefox specific code branches:

  • make FF use contentEditable (like other browsers) rather than designMode
  • make FF use nested <div> inside of <body> (inside of <iframe>), just like other browsers
  • expand tabbing helper code (both for tabbing/shift-tabbing into and out of the iframe) to run on all browsers, not just IE
  • reduce the number of branches in focus(), so that Chrome and FF use same path

Fixes #10415, #17560.
Refs #11898.

(cherry picked from commit 0854ec27436ba935b09e29c0cfc2960656780070)

Conflicts:

_editor/RichText.js

comment:15 Changed 15 months ago by mahays0 <mahays0@…>

In a443c8477ce0111edffe0977dbe5794380af1001/dijit:

Fixes and refactorings for Editor on Firefox.

Fixes Editor_a11y.html regression test failure and removes firefox specific code branches:

  • make FF use contentEditable (like other browsers) rather than designMode
  • make FF use nested <div> inside of <body> (inside of <iframe>), just like other browsers
  • expand tabbing helper code (both for tabbing/shift-tabbing into and out of the iframe) to run on all browsers, not just IE
  • reduce the number of branches in focus(), so that Chrome and FF use same path

Fixes #10415, #17560.
Refs #11898.

(cherry picked from commit 0854ec27436ba935b09e29c0cfc2960656780070)

Conflicts:

_editor/RichText.js

(cherry picked from commit f7e337959c313ccfdeae8e26c629a48e60fa9ed0)

Conflicts:

_editor/RichText.js

comment:16 Changed 10 months ago by bill

  • Type changed from enhancement to task

Changing to category "task" since presumably this has no impact on users.

comment:17 Changed 10 months ago by bill

  • Milestone changed from 1.10 to 1.8.7

comment:18 Changed 6 months ago by Bill Keese <bill@…>

In 85084afe0860604414984acf95fb5edcb84cad19/dijit:

Fixes and refactorings for Editor on Firefox.
Somehow this also fixes the LinkDialog regression failure on IE11.
Fixes Editor_a11y.html regression test failure and removes firefox specific code branches:

  • make FF use contentEditable (like other browsers) rather than designMode
  • make FF use nested <div> inside of <body> (inside of <iframe>), just like other browsers
  • expand tabbing helper code (both for tabbing/shift-tabbing into and out of the iframe) to run on all browsers, not just IE
  • reduce the number of branches in focus(), so that Chrome and FF use same path

Fixes #10415, #17560.
Refs #11898.

(cherry-picked from a443c8477ce0111edffe0977dbe5794380af1001)

comment:19 Changed 6 months ago by bill

  • Milestone changed from 1.8.7 to 1.7.6
Note: See TracTickets for help on using tickets.