52868 - clicking below table should not put selection inside table

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[sujay]

using 9/15 build of netscape

1) launch netscape
2) launch composer
3) insert default table

Now Mike fixed the issue with being able to insert caret before or after a
table...but what if the user wants to be able to insert caret "below"
the table....you can't do it..workaround is to place caret after the table
and then hit return a couple times.

all platforms.

see 4.x behavior....

Comment 1

[anthonyd]

I can quite easily reproduce this, but I highly doubt it will be fixed for rtm, 
as the workaround is sooooo easy, and intuitive.

anthonyd
setting to future.

Comment 2

[Charles Manske]

This works if you click *after* the end of the table (in the right border).
You will see a caret at the lower left corner of table and typing there 
starts new line.
Tested on Windows using debug build, mozilla tree, 9/26.
Sujay: Please test on all platforms using commercial builds.
So if we want to support clicking *under* the table, that is an RFE

Comment 3

[Charles Manske]

This works if you click *after* the end of the table (in the right border).
You will see a caret at the lower left corner of table and typing there 
starts new line.
Tested on Windows using debug build, mozilla tree, 9/26.
Sujay: Please test on all platforms using commercial builds.
So if we want to support clicking *under* the table, that is an RFE

Comment 4

[shrirang khanzode]

I almost ended up filing another bug for this problem. I was not able to guess 
the workaround (since my table width was 100%) or maybe I am too dumb :). Can 
this bug be reconsidered for fixing? 

Comment 5

[Kathleen :Brade]

correct summary to use "caret"

Comment 6

[sujay]

Can we nominate this for nsbeta 1..

Lots of people complaining about this behavior...

Also we have a whole bunch of bugs filed in the past on this problem:

45488, 46715, 34356

Comment 7

[anthonyd]

the problem with this bug, is that to fix this we will have to insert an 
element after the table in order to have a place to put the caret.

adding muudge and beppe, maybe they have some ideas to fix this.  again, its not 
 that this is a ahard bug, its just that we need the best and PROPER situation.

anthonyd

(possible 0.9 candidate)

Comment 8

[anthonyd]

taking

Comment 9

[shrirang khanzode]

am i seeing things..? is this working on windows already ?

Comment 10

[shrirang khanzode]

oh...does not work if the table is the only thing on a page...

Comment 11

[rubydoo123]

could we add a moz-dirty break after the </table> tag? then strip it on output 
and insert on input? how much of a perf hit would that be?

Comment 12

[rubydoo123]

what needs to be fixed here is when the user seelcts/clicks below the table the 
caret should appear to the right of the table and not in the cell. Reassign to 
mjudge and setting to moz0.9.1

Comment 13

[anthonyd]

i think mike is supposed to have this per beppes last comments.
anthonyd, but i dont know what he is going to do as the problem is that nothing 
exisits below the table to palce the caret in *shrug*.  we got around this in 
4.x by using a moz_br fake tag.

anthonyd

Comment 14

[rubydoo123]

After talking to Kin and Anthony, Joe will need to fix this one due to it
involving the rules code. I believe we should be adding a br after a table (just
one br) so there is a selection point after the table, with the br in place,
selecting below the table will snap to the br and not within the table.

Comment 15

[Charles Manske]

This is a very old issue. There are many that really hated Composer for the
BR we forced before and after a table when it is the only thing in a page.
I certainly understand how much easier this problem is to solve by adding the
BR after a table -- can we add "-moz-br" that is stripped out when we save the
file?

Comment 16

[rubydoo123]

the moz-br would be ok on creation, but what if you brought a file back in and
the table was the last item? or as Joe pointed out, what if two tables are
inserted without text in between

Comment 17

[Charles Manske]

Exactly: "adjacent tables" is a big problem. People want to use tables for
layout, and often use nested tables. Putting in *any* extra BRs really messes
that up.

Comment 18

[rubydoo123]

just talked with Joe, says this should go back to mike

Comment 19

[rubydoo123]

ok, I'm finally on track with this issue! The br after the table is not part of 
this bug! I am opening a new bug for that and assigning to Joe. The real issue 
for this bug is the original complaint, being seelcting under a table puts caret 
in a cell at the bottom of the table -- that should not happen.

Comment 20

[mjudge]

ok I CANT allow clicking below a table NOT to snap into it. here is why.
as someone pointed out earlier people use tables as a layout feature.  when in 
the browser, if you click below a table you want it to behave as though the 
table was just that.  selecting below content and sliding the selection left or 
right as you move is a windows standard we have to keep even with tables around.

OTHER SOLUTION:
I have a fix i need to make for some other bug that gives an added side effect. 
I know windows for example (and I believe unix and mac also) have a 
"CompleteMove" feature.  On windows its "CTRL+HOME or CTRL+END".  I cant 
remember the other platforms.  anyway..  now if say a table was the only thing 
on the page, you could "click" to the right/left of the table as before AND you 
could also hit "CTRL+END" or "CTRL+HOME" to jump before or after that table.  
This is a more discoverable feature i believe.  
    I also want to make the "down/up/left/right" arrows also take you outside 
the table, but that is not ready yet.  
    Here is the patch so far.  I think it makes it better than it was.  We can 
leave this bug open to make the arrow keys take us outside the table for later. 
(maybe i can get this in by the end of the weekend who knows...)  In either case 
I am looking for REVIEWER and APPROVAL with the caveat that I will not check it 
in if my solution (partial or otherwise) does not get some consensus here.

Comment 21

[mjudge]

Attachment: patch for "COMPLETE MOVE" in presshell

Comment 22

[mjudge]

ok now DOWN and UP arrows at bottom or tops of tables will snap out of the table 
to before/after.  this is new set of diffs. nsPresShell.cpp , nsSelection.cpp 
and nsGfxTextControlFrame.cpp.  I think this is fixed now.

Comment 23

[mjudge]

Attachment: SET#2 nsSelection.cpp patch to not do default failure at suck a low level

Comment 24

[mjudge]

Attachment: SET#2 nsGfxTextControlFrame.cpp: handle failure of line move here. simple

Comment 25

[mjudge]

Attachment: SET#2 SamePresShell diff except we now need to handle failure of line move here

Comment 26

[mjudge]

Attachment: SET#2 BAD GFXTEXTCONTROL PATCH BEFORE oops. bad char got in there

Comment 27

[kinmoz]

nsSelection.cpp changes (06/16/01 11:58 attatchement):  
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38753

  * Does removing the code at this point regress bug 46811?



nsPresShell.cpp changes (06/16/01 12:01):  
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38755

  * Why are we calling CompleteMove() when an error occurs? Can you
    add a comment that explains what's going on?

  * There's an unused "nsFrame *frame" var that was added.

  * I noticed you removed the null check for nodeList ... are we
    guaranteed, by all implementations, that we always get a non-null
    nodeList if no error is thrown from GetElementsFromTagName()?

  * In CompleteMove() is there any way that you can get into an
    infinite do() loop if an error is thrown by the nsILineIteratorNavigator QI?

  * This is the same fix for bug #82101 right?

Comment 28

[Simon Fraser [no longer active]]

Any way to get a single diff file for all the changes?

I'm also somewhat confused about what these changes are doing, and whether they 
are in the correct bug.

Comments:
Attachment 38755 [details] [diff]

+  if (NS_FAILED(result))
+    result = CompleteMove(aForward,aExtend);

It's not obvious what 'CompleteMove' does from the name (though I know you didn't 
add it in this patch). Does it move to the start/end of the line? To the start/
end of the document? A comment would help.

+          }
+          do

Blank line here please.

+            //check to see if this is ANOTHER blockframe inside the other one if 
so then call into its lines
+            result = pos.mResultFrame->
QueryInterface(NS_GET_IID(nsILineIteratorNavigator),getter_AddRefs(newIt));

Maybe be paranoid here and check for a non-null pos.mResultFrame?

Comment 29

[rubydoo123]

Mike - why can't you snap to below the table? You stated:
   ok I CANT allow clicking below a table NOT to snap into it. here is why.
   as someone pointed out earlier people use tables as a layout feature.  when  
   in the browser, if you click below a table you want it to behave as though   
   the table was just that.

You are talking about browsing functionality here, not editing. The issue is on 
edit, the caret needs to appear below the table, if the user selects outside the 
table then the expectation is to have the caret fall below it, not land in a 
cell.

Comment 30

[kinmoz]

sr=[e-mail], r=[e-mail] with the changes discussed/mentioned
above.

Comment 31

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 32

[mjudge]

fix is in but i will leave this open since i left out the "editor specific" fix

Comment 33

[rubydoo123]

this is quite a common procedure

reviewed and approved

Comment 34

[sujay]

Joe put in <br> after table, so now the caret appears after
the table..

so this issue is fixed.

verified in 7/9 build.

Comment 35

[rubydoo123]

fixed

Comment 36

[sujay]

verified