[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 #8 from kalman at chromium.org  2010-10-25 04:43:44 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.

Ah, thank you.

I've added the argument to start/endOfParagraph, and a token test (for the indentation) which did indeed crash after removing the check.  It seems a little... messy... but perhaps that's inevitable.

>> WebCore/editing/visible_units.cpp:816
>> +        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.

No idea -- would this case be different to the same check in endOfParagraph?

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