Closed Bug 677269 Opened 8 years ago Closed 8 years ago

Set `selection.text` or `selection.html` having multiple selection, ends up to corrupt the selection

Categories

(Add-on SDK Graveyard :: General, defect, P2)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zer0, Assigned: zer0)

References

Details

(Whiteboard: [fixed by bug 619273 / bug 698237])

Attachments

(1 file)

In case of multiple selection on the same node, set `selection.text` or `selection.html`, will collapse all the selection below the one modified (usually the first one, returned by default by selection module), to the start offset of the modified one.
Also, the selection is now corrupted, showing everything selected from the start of the container node to the start of the modified selection offset.

This behavior in the Addon SDK is the direct consequence of bug 673108.
Assignee: nobody → zer0
Depends on: 673108
Group: core-security
I suggest to have as expected behavior something like: https://bug673108.bugzilla.mozilla.org/attachment.cgi?id=547391

That should also fix some issues that we have at the moment:

- Set the content of a single selection makes the text unselected

- Set the content of a selection create an unnecessary new span element in the DOM tree as container

In order to give the capability to the addon developers to don't display the selection, I suggest to implement a `selection.collapse` method.

Also, at the moment set `selection.html` or `selection.text` is the same, I'd like to take this bug to force the "text" version of the string if we set the selection.text. It means, if we set:

selection.text = "<b>Foo</b>";

what we will have in the HTML will be a text with "&lt;b&gt;Foo&lt;/b&gt;" instead of a "Foo" in bold.
Matteo, how often do you think people will hit this? Seems like it might be a rare case
Whiteboard: [triage:followup]
We can definitely say that, multi selection in general is not a common scenario in my opinion – only Firefox support it, apparently, Chrome and Safari on Mac doesn't have it.
Not only users, but also some developers doesn't know that they can actually have discontiguous selections.

However, notice that the current behavior it's really messed up, and this fix also correct a mistake was done with the single selection as well: in according to Myk, when we change the content of a selection, the text should be still selected, but it's not, because the current implementation doesn't take in account the same issue we have for multiple selections.

Plus, several minor fix like a tidy DOM tree – now everytime we change the content of a selection, we add a new span node – and a `selection.text` that works properly, and not like an alias of `selection.html`.

I basically discovered these things working on bug677269, and partially already fixed.
Priority: -- → P2
Whiteboard: [triage:followup]
Target Milestone: --- → 1.2
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: 1.2 → ---
Attachment #551475 - Attachment mime type: text/plain → text/html
Attachment #551475 - Attachment mime type: text/html → text/plain
Does this bug still occur in the latest Nightly build?
All issues should be fixed by bug 619273 / bug 698237 in the latest Nightly builds:
http://nightly.mozilla.org/

Please file a new bug and CC me if you still see some error.  Thanks for your report!
Status: NEW → RESOLVED
Closed: 8 years ago
Depends on: 619273, 698237
Resolution: --- → FIXED
Whiteboard: [fixed by bug 619273 / bug 698237]
You need to log in before you can comment on or make changes to this bug.