Open Bug 85505 Opened 23 years ago Updated 3 years ago

Frame code and Caret code need work to allow caret in more places

Categories

(SeaMonkey :: Composer, defect, P3)

Tracking

(Not tracked)

People

(Reporter: mozeditor, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: embed, perf, topembed-, Whiteboard: EDITORBASE-)

Currently we cannot get a caret on:
1) blank lines caused by newlines in a preformatted context
2) empty list items and table cells

I've been told that this is because the caret code depends on a frame being
available, and there isn't any in these contexts.

Frames people and caret people need to get together and fight the good fight.
Item 1 may be more common as caused by bug
http://bugzilla.mozilla.org/show_bug.cgi?id=67334.

(beppe referred me to this bug report when I showed her a problem I had with not
being able to click on parts of my compose window to edit / type my message.)
Bug 67334 is a separate bug having to do with output.
*** Bug 92513 has been marked as a duplicate of this bug. ***
Reassigning to mjudge. table cells have frames. attinasi and waterson can help 
with pre and lists as needed.
Assignee: karnaze → mjudge
mike, it has come full circle! can you please explain to Chris why this is a 
layout bug and not an editor bug
Priority: -- → P2
Target Milestone: --- → mozilla0.9.4
Blocks: 92921
This bug becomes a blocking factor for correctness bugs in Composer. Because
of this bug, new AllTags mode has to generate <br> into <style> and <script>
elements instead of line breaks, which is obviously an mistake. We also have
to generate <br> inside <pre>, which is not the expected behavior. We also
cannot display the <br> tag in AllTags mode because of this bug.

attinasi, waterson : pliiiiiiiiiizzzzz :-)
This bug becomes a blocking factor for correctness bugs in Composer. Because
of this bug, new AllTags mode has to generate <br> into <style> and <script>
elements instead of line breaks, which is obviously an mistake. We also have
to generate <br> inside <pre>, which is not the expected behavior. We also
cannot display the <br> tag in AllTags mode because of this bug.

attinasi, waterson : pliiiiiiiiiizzzzz :-)
So, how is this going to work? *thinks*

To get accurate caret placement, layout can do one of two things:
 i) When the caret is placed in a particular location, we generate some anonymous
    content that acts as a placeholder for the caret. Layout should thus reflow
    the document to show correct caret placement. As soon as the user types,
    inserts real content, or moves the selection away, that anon content is
    destroyed.

ii) Layout is able to supply the caret with coordinates that tell it "if there
    was content here, this is where it would be".

The big problem here is the very act of inserting bogus content to give the caret 
somewhere to draw causes the document layout to change. We've hacked around this 
with <br>s, to try to give the user a nice cozy feeling of being inside a word 
processor. But for "real" HTML editing, we need to come up with a scheme that 
works in situations where there may not be any vertical space to draw a vertical 
blinking bar. We may have to use a horizontal blinking bar in some situations, or 
some other solution.
Blocks: 95874
remove milestone since mjudge is on sabbatical and not likely to fix before 0.9.4 
is long gone
Target Milestone: mozilla0.9.4 → ---
*** Bug 98421 has been marked as a duplicate of this bug. ***
*** Bug 97195 has been marked as a duplicate of this bug. ***
mjudge: has this fallen off your radar because the milestone is unset?

Gerv
So what decides where the caret goes?  And why is it a problem to generate a
caret in the places mentioned above?  Is the only relevant code
nsCaret::GetCaretCoordinates?
In my jedgement, this is EDITORBASE
Whiteboard: EDITORBASE
Syd elevates this to P1 based on akkana's commentary over e-mail, to whit:

Joe Francis writes:

> Selection needs to highlight a non-zero width rectangle when a br node
> is selected.  This is so users can select blank lines, and also so that
> they can tell if they have selected a trailing br at the end of a line
> of text.  The later is important so that they know if a following line
> will be pulled up if they delete.

In case anyone is interested in tracking the issue:

Note that this is closely related to and dependant on bug 85505 (the
only difference is that I don't think that bug makes it clear that the
selection should be visible and nonzero width -- it only asks that
it be possible to place a caret there).  Is there a bug on showing
a selection, not just a caret, or should it be made a part of that bug?

Fixing this would solve so many editor problems, and allow us to remove
so much performance-destroying code (i.e. it would improve editor
and mail compose performance dramatically) that it's hard to list
all the benefits.

Priority: P2 → P1
setting a milestone for this EDITORBASE bug
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
Keywords: topembed
Joe we tried a couple of things:

We typed in a list, 3 non-blank lines, selected the middle one, hit delete to
remove the content. I can click away, then click back.

Next, just typed in blank lines in preformatted text and were able to click
around OK. We were able to position caret no problem.

EDITORBASE triage team wants some specific cases that can exhibit this problem
and determine who is impacted by it.
You were able to do these things because the editor jumps through a LOT of hoops 
to make them work.  Your empty list items get populated with a single <br> 
whenever they become empty.  Your preformatted text had it's newlines relaced 
with <br>s.  If we could put a caret in empty list items, etc, and on empty 
preformatted lines caused by newlines, then the editor wouldn't have to do so 
much gyrating in response to user edit actions.  Being able to remove this logic 
from the editor would result in smaller footprint, *much* greater performance in 
certain areas, and greater maintainablilty/fewer regressions.
All sounds good, but can we push this past mozilla 1.0 and EDITORBASE- ?
No one answered dbaron's questions in comment #13.  Why not?

syd: why shouldn't we fix these problems sooner (0.9.9 or even 1.0) rather than
later (after nsbeta, too late to get a milestone or 1.0-release-candidate
talkback cycle)?

/be
I'm the worng person to address dbaron's questions, but I'll try anyway (sfrasre 
wrote caret and can give better answers).  I believe that we need a frame for 
more reason than just getting coordinates.  I think that Simon found that he 
needed a frame in order to prevent "caret turds".  I don't know the details why 
that is so.  Simon?
To respond to dbaron:
Caret positioning is affected by two things:
1. User-expectations of editing behaviour, in line with other word processors.
   In other words, we can't always draw the caret in the place specified by the
   collapsed selection; we need some special cases where the caret is drawn with
   a different height and/or position that you would assume given simply a
   collapsed DOM range. (Such problems manifest themselves with an "uber-caret".)

2. Interaction with layout drawing code.
   Currently, the caret is restricted to drawing inside of (leaf?) frames. Any
   time that we've attempted to draw the caret outside of frames, we've ended
   up with lots of caret turd issues. Drawing inside of frames is the only way
   we've been able to predict with certainty when the caret will be erased and
   require redrawing.

It's quite possible that we could solve these drawing issues with a caret
redesign, but it's not trivial.
removing myself from the cc list
Target Milestone: mozilla1.0 → mozilla1.0.1
Keywords: topembedembed, topembed-
EDITORBASE-
Whiteboard: EDITORBASE → EDITORBASE-
Target Milestone: mozilla1.0.1 → mozilla1.1alpha
Is any progress being made on this issue?

I want to reiterate the complaint made in Bug 98421 (editor: Core), which has
been marked as a duplicate of this bug.

The HACK of putting a <BR> in every empty table cell changes the way tables are
rendered, because every row is now at least one text line high, (unless every
cell contains something smaller--like a 1-pixel gif, which takes a lot of code!)

This means that any time I resave someone else's page in Composer the first
time, it can CHANGE THE RENDERING OF THE PAGE PERMANENTLY.  THIS MAKES COMPOSER
NEARLY USELESS TO ME!

(It's also very annoying that my source code gets reformatted because a hard
return is added after the BR, placing the </td> on a separate line by itself!)

I think 98421 should be reopened and made dependent on 85505.

(Can someone suggest something I can put in a cell that will not increase its
height, and take less code than an IMG tag?)
Jim booth writes:
>(It's also very annoying that my source code gets reformatted because a hard
>return is added after the BR, placing the </td> on a separate line by itself!)

You should open new bug on that against the component "DOM to Text Conversion".  
What should probably happen there is that newlines are only forced after a <br> 
if the content following the <br> is not a close tag.
JFrancis writes:
>You should open new bug on that against the component "DOM to Text Conversion".  
>What should probably happen there is that newlines are only forced after a <br> 
>if the content following the <br> is not a close tag.

That problem (extra hard returns added to my code) is a miniscule annoyance
compared to the EXCESS <BR>s inside every blank table cell, and elsewhere!  And
it will become moot when this one is fixed.   I'm not going to open that bug.

THIS BUG MAKES IT VERY DIFFICULT FOR ME TO CONTINUE USING COMPOSER AT ALL!   IS
IT GOING TO BE FIXED SOON?  What good does a target of mozilla 1.1alpha,
severity of major, and a priority of P1 do?

Added perf keyword because this issues affects editor performance.

Fixing this would require a big layout change.
Keywords: perf
Blocks: 174361
Blocks: 240933
*** Bug 255497 has been marked as a duplicate of this bug. ***
*** Bug 314519 has been marked as a duplicate of this bug. ***
Blocks: 314519
Status update:

The fixes for bug 298690 and bug 314519 took care of placing the caret in empty lines in preformatted context (298690 fixed mouse-click and up/down arrows, 314519 fixed left/right arrows).

The major dependencies of this bug (namely bug 92921 and bug 240933) only actually depend on fixing the "blank lines caused by newlines in preformatted context" issue, so there's nothing (here) stopping us from fixing them now.

As for the remaining issues (empty list items and empty table cells):
I can place the caret in these positions using left/right arrows, but not using mouse-click and up/down arrows.
I'm not sure whether it's worth keeping this bug open for those issues, or if we should open a separate bug for them.

Note that the premise in comment #0 ("I've been told that this is because the caret code depends on a frame being available, and there isn't any in these contexts") isn't true (in all of these contexts there is a frame), and I doubt if it was even true at the time the bug was filed.
Assignee: mjudge → nobody
No longer blocks: 314519
Status: ASSIGNED → NEW
Depends on: 298690, 314519
QA Contact: chrispetersen → layout
Depends on: 287813
No longer blocks: 240933
Priority: P1 → P3

I think this belongs to SeaMonkey Product now, moving this over. If this is not the correct Product/Component please toss it back or move it to the correct one.

Component: Layout → Composer
Product: Core → SeaMonkey
Target Milestone: mozilla1.1alpha → ---
You need to log in before you can comment on or make changes to this bug.