[Webkit-unassigned] [Bug 48037] Triple click does not select whole line for mixed contenteditable regions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48037





--- Comment #6 from Ryosuke Niwa <rniwa at webkit.org>  2010-10-22 13:11:24 PST ---
(From update of attachment 71385)
View in context: https://bugs.webkit.org/attachment.cgi?id=71385&action=review

>>> WebCore/editing/visible_units.cpp:-801
>>> -            break;
>> 
>> Could you elaborate on why it is okay to remove this check?  It's not too obvious to me that editable boundaries are respected without this check.
> 
> I'm not sure what you mean.  In what way should editable boundaries be respected?
> Elaboration: it's this check that incorrectly stops the selection when a non-contenteditable region is encountered inside a contenteditable one.  I say "incorrectly" because both IE and FF select across them.  From what I can tell there is sanity checking elsewhere (VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries).

startOfParagraph is used everywhere in editing and removing this check would allow editing commands to operate across editing boundaries.  e.g. Suppose we have <div>hello <span contenteditable>world</span></div>, and "world" is selected.  When we indent, we shouldn't be indenting "hello".

You're probably trying to fix the behavior of startOfParagraph when called in VisibleSelection::setStartAndEndFromBaseAndExtentRespectingGranularity.  That's very specific use of startOfParagraph and even for that we need to verify that it won't break other things.  Now, setStartAndEndFromBaseAndExtentRespectingGranularity is called by validate with granularity != CharacterGranularity in expandUsingGranularity.  There are 4 calls to expandUsingGranularity, and all of them seem to be user-initiated so they're fine.  The call from expandSelectionToGranularity seems fine as well because they're executing commands to extend selection.

So we can probably modify the behavior of setStartAndEndFromBaseAndExtentRespectingGranularity(ParagraphGranularity) without breaking anything. And if we're changing the behavior of startOfParagraph and endOfParagraph for these two functions, then we probably want to add an argument to these functions to ignore editing boundaries.  See Position::CanCrossEditingBoundary.

> WebCore/editing/visible_units.cpp:816
> +        // FIXME: We avoid returning a position where the renderer can't accept the caret.
> +        // We also do this in endOfParagraph, and might want to do it in other cases too.
> +        if (r->isText() && r->caretMaxRenderedOffset() > 0) {

Are you sure this won't skip unicode-bidi control characters, zero-width space, etc... ?  I need Xiaomei's or Dan's confirmation on this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.