Open Bug 196774 Opened 22 years ago Updated 2 years ago

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

Categories

(Core :: DOM: Selection, defect)

defect

Tracking

()

People

(Reporter: kinmoz, Unassigned)

References

Details

(Keywords: topembed+, Whiteboard: EDITORBASE+)

Attachments

(1 file, 5 obsolete files)

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.
Whiteboard: EDITORBASE
Nominating topembed since this is needed to complete the atomic selection feature delivered to one of our embedding clients.
Blocks: PhtN5
Keywords: topembed
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.
Status: NEW → ASSIGNED
Attached patch patch for nsFrame.cpp (obsolete) (deleted) — Splinter Review
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.
Attached patch patch for nsFrame.cpp (obsolete) (deleted) — Splinter Review
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 patch.
Attachment #117112 - Attachment is obsolete: true
Attached patch patch for nsBlockFrame.cpp (obsolete) (deleted) — Splinter Review
small patch to have the handleevent code abort when the getprevline call triggers that it hit an inline stop (for links)
Attached patch patch for nsFrame.cpp (obsolete) (deleted) — Splinter Review
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
Attached patch nsTextFrame.dif (obsolete) (deleted) — Splinter 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.
Keywords: topembedtopembed+
Whiteboard: EDITORBASE → EDITORBASE+
When is this landing on the trunk? Also, are the patches available ready to land on the ANGELON branch?
Attached patch inclusive patch (deleted) — Splinter Review
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
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 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 up 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.
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
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.
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?
Assignee: mjudge → sharparrow1
Status: ASSIGNED → NEW
QA Contact: pmac → selection

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

The bug assignee didn't login in Bugzilla in the last 7 months.
:hsinyi, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: sharparrow1 → nobody
Flags: needinfo?(htsai)
Flags: needinfo?(htsai)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: