[Webkit-unassigned] [Bug 106817] [CSS Regions] Selecting text inside + outside regions does not work properly

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Javier Fernandez <jfernandez at igalia.com>  2013-07-10 10:42:39 PST ---
(In reply to comment #12)
> (From update of attachment 206131 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206131&action=review
> 
> > Source/WebCore/ChangeLog:1
> > +2013-07-05  Javier Fernandez  <jfernandez at igalia.com>
> 
> This is an informal review, which you may/may not take into account. You need somebody with more editing knowledge to review it. Also, it may be interesting to understand the impact on performance introduced by this code. Also, i see that there are tests that fail and they also fail on my machine after i applied your patch - can you post a patch without the failures?

I can confirm such tests failing in my machine as well, my bad. The thing is that the "plainText" function is widely used to the get the Element inner text, for instance, so ew can't touch that function. 

I've got a patch already that solves the issue (just the issue reported in this bug) without causing any regression, but the approach is to modify the Range::toString instead, in order to skip nodes without renderer and belonging to different Flow Threads. 

I think modifying the range::toString function is not an option; while not causing any regression, it looks like it follows the Mozilla standards and in such browsers the Range::toString function gets "not-rendering" content as well, so perhaps that's the expected behaviour. 

I'm exploring now the multi-range selection approach, which, even though being more challenging and ambitious, I think it's the right approach to follow.

Also, I agree on the performance issues; I'll focus on them once I've got the valid solution to solve the general problem of Selection with Regions, not just the case reported here.


> 
> > Source/WebCore/ChangeLog:7
> > +
> 
> I would like to see a description of what this patch is fixing and the approach taken. I do not think it is fixing the whole selection in regions. Maybe we can use 106817 as a master bug and add this patch and other following patches to this master bug?

Yes, I think the right choice is to select this bug as "meta-bug" for all the selection related issues involving Regions.  

It's also true that this patch does not solve all the Selection issues when using Regions, but it does it for many of them. I'm starting to realize that we need multi-range or something like that to solve all the issues, but that would require further discussion. I hope to get a preliminary patch soon to start the discussion.

> 
> > Source/WebCore/ChangeLog:23
> > +        at editing side.
> 
> Can you please explain why?

Well, the patch for 105641 just avoids rendering selections including nodes of different Flow Threads; it doesn't face the nested regions issue specifically, but any other case involving selection of contents from different regions.

Besides, the selected range is not correct, so getting the selected content would lead to incorrect results. Fixing the issue at editing side would solve both angles, selection and rendering. Actually, my patch solves both bugs. 

I came to the conclusion that Selection is a editing matter; it generates Ranges which are used for different purposes, one of them, rendering the selected content.

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