Can insert caret into -moz-user-select:all subtree that wraps across several lines

Assigned to



16 years ago
9 years ago


(Reporter: kinmoz, Assigned: Eli Friedman)




Firefox Tracking Flags

(Not tracked)


(Whiteboard: EDITORBASE+)


(1 attachment, 5 obsolete attachments)



16 years ago
If you have a -moz-user-select:all subtree that wraps and spans 2 or more lines,
you can actually place the caret into the subtree by clicking to the right
(roughly where the y coordinate of the baseline would be) of the 1st line
containing the subtree.

This totally freaks out the editor. Any text you enter at that point will appear
on the last line that the subtree wraps to, and even if you've typed several
characters, a single backspace over any char you just entered will result in the
entire -moz-user-select:all subtree disappearing.


16 years ago
Whiteboard: EDITORBASE

Comment 1

16 years ago
Nominating topembed since this is needed to complete the atomic selection
feature delivered to one of our embedding clients.
Blocks: 193020
Keywords: topembed

Comment 2

16 years ago
yes i know about this. it has to do with jumping outside of a link before it
jumps outside the atomic selected stuff. I will have a fix posted by end of the day.

Comment 3

16 years ago
Created attachment 117112 [details] [diff] [review]
patch for nsFrame.cpp

this does a check to make sure we dont go into the link specific code.	If we
dont do this check then the link code detects that its in a special link case
and "fixes" selection and leaves before the display all code can be triggered.

Comment 4

16 years ago
Created attachment 117122 [details] [diff] [review]
patch for nsFrame.cpp

this bigger patch also fixes home-end issues. I just realized you cant have the
previous patch work without this code as well.	I will also add an nsBlockFrame
Attachment #117112 - Attachment is obsolete: true

Comment 5

16 years ago
Created attachment 117123 [details] [diff] [review]
patch for nsBlockFrame.cpp

small patch to have the handleevent code abort when the getprevline call
triggers that it hit an inline stop (for links)

Comment 6

16 years ago
Created attachment 117260 [details] [diff] [review]
patch for nsFrame.cpp

combines up/down/home/end/left/right into one check for USER_SELECT_ALL this
saved space and also irregularities.  if peek offset ends up in atomic
selection, continue until you are no longer in an atomic selection.
Attachment #117122 - Attachment is obsolete: true

Comment 7

16 years ago
Created attachment 117266 [details] [diff] [review]

use this in addition to the nsFrame code.  This will prevent accidental carat
movement into the select all style.  This also uses a common place to catch all
the edge cases.


16 years ago
Keywords: topembed → topembed+

Comment 8

16 years ago
When is this landing on the trunk?  Also, are the patches available ready to
land on the ANGELON branch?

Comment 9

16 years ago
Created attachment 117658 [details] [diff] [review]
inclusive patch

path from top of layout. this should be up to date
Attachment #117123 - Attachment is obsolete: true
Attachment #117260 - Attachment is obsolete: true
Attachment #117266 - Attachment is obsolete: true

Comment 10

16 years ago
20030327 Trunk build:

I have noticed that you can arrow around the bounds of the span object and enter
text within the moz-user-select:all subtree as well.

In particular, after a drop of the subtree, the cursor is placed to the right of
the element after the anchor element but before the closing </span> tag.

Comment 11

16 years ago
Comment on attachment 117658 [details] [diff] [review]
inclusive patch

It looks like this patch can cause us to blow the runtime stack in the case
where  the last thing in the document is a -moz-user-select:all subtree, and
you click below it in the empty content area of the body.

The problem seems to be that when we click below all of the content, the event
gets dispatched through the body frame's nsBlockFrame::HandleEvent() method,
which then calls nsFrame::GetNextPrevLineFromeBlockFrame() which returns a
pos.mResultFrame pointer that points at the body frame, so we effectively end
calling pos.mResultFrame->HandleEvent() later in the method causing us to
infinitely recurse.

The problem seems to be that GetNextPrevLineFromeBlockFrame() initially
calculates an pos.mResultFrame that is inside a -moz-user-select:all subtree so
then it executes this code that was added:

+    do 
+    {
+      currentFrame = parentFrame;
+      currentFrame->GetParent(&parentFrame);
+      if (parentFrame)
+	 parentFrame->IsSelectable(&selectable, &parentStyle);
+    }
+    while (parentFrame && (parentStyle == NS_STYLE_USER_SELECT_ALL));

which ends up crawling up the frame hierarchy until we hit the body again. The
code after this then resets pos.mResultFrame to parentFrame and returns.

Comment 12

15 years ago
if any branch needs this earlier I can check the code in as well to 1.4b/final.
 This will take a little time to do right and I dont want to cause a regression
before vacation.
Target Milestone: --- → mozilla1.5alpha

Comment 13

15 years ago
any other owners for this?
unsetting target milestone
Target Milestone: mozilla1.5alpha → ---
Eli, you might be interested in this. It seems like your patch to bug 316281 somewhat improved the situation here (typing characters adds them at the end of the -moz-user-select:all element, and they can be deleted normally), but the basic issue is still there.

Comment 15

13 years ago
The "basic issue" being that caret drawing doesn't account for inline continuations?  That seems relatively easy to fix by checking for continuations before drawing the caret for a non-text frame (the solution in the patch for this bug doesn't look like it would work correctly).

There's also the issue of bugs related to keyboard navigation and dragging -moz-user-select-all trees.  The patch bug 316281 actually prevents up/down from going into -moz-user-select-all trees, although it breaks getting past them.  I think the best approach to fixing the many issues related to -moz-user-select: all and the keyboard would basically be cleaning up PeekOffset (I forget the bug number).  I'm not quite sure what the drag path does; that would take a bit of research.

Skimming the patch, it doesn't look useful, and it looks like it introduces some bugs, although I could be wrong.

I think it would be best to split the caret drawing issue off into another bug, and do the actual work in other bugs.  This would then become a sort of tracker for the issues brought up in this bug so we don't lose track of the problems.  Should I go ahead and do that, or do you have a different idea?


13 years ago
Assignee: mjudge → sharparrow1
QA Contact: pmac → selection
You need to log in before you can comment on or make changes to this bug.