Bug 119622 - [CSS Regions] Not possible to clear the selection when mixing content from different FlowThreads
Summary: [CSS Regions] Not possible to clear the selection when mixing content from di...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 106817 109283
  Show dependency tree / graph
 
Reported: 2013-08-09 05:15 PDT by Javier Fernandez
Modified: 2013-09-16 04:20 PDT (History)
13 users (show)

See Also:

Attachments
Patch (5.08 KB, patch)
2013-08-09 06:10 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Patch (5.08 KB, patch)
2013-08-09 06:34 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Patch (5.77 KB, patch)
2013-08-09 12:07 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Patch (10.17 KB, patch)
2013-08-14 15:30 PDT, Javier Fernandez 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Javier Fernandez 2013-08-09 05:15:59 PDT 
This bug was hidden by the fix for the bug 105641, which basically prevented the selection to be rendered when crossing the FlowThread boundaries. 

Such fix is not correct, because instead of fixing the actual bug it adds restrictions on how the selection works with Regions.

We need to properly fix the selection clearing operation, allowing selections to cross RenderFlow boundaries and select content from different Regions.
Comment 1 Javier Fernandez 2013-08-09 06:10:21 PDT 
Created attachment 208425 [details]
Patch
Comment 2 Build Bot 2013-08-09 06:13:30 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1353447
Comment 3 Build Bot 2013-08-09 06:14:33 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1405199
Comment 4 EFL EWS Bot 2013-08-09 06:15:04 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1411318
Comment 5 Early Warning System Bot 2013-08-09 06:15:55 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1369686
Comment 6 Early Warning System Bot 2013-08-09 06:16:04 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1414328
Comment 7 EFL EWS Bot 2013-08-09 06:16:12 PDT 
Comment on attachment 208425 [details]
Patch

Attachment 208425 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1369685
Comment 8 Javier Fernandez 2013-08-09 06:34:42 PDT 
Created attachment 208429 [details]
Patch
Comment 9 Build Bot 2013-08-09 06:38:30 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1385270
Comment 10 Build Bot 2013-08-09 06:41:31 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1369695
Comment 11 Sergio Villar Senin 2013-08-09 07:31:50 PDT 
Comment on attachment 208429 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208429&action=review

Not really my field of expertise, just doing an informal general review.

> Source/WebCore/rendering/RenderView.cpp:782
> +    bool cond = os && os != stop;

Do not use "cond", use a more descriptive name.

> Source/WebCore/rendering/RenderView.cpp:849
> +    cond = o && o != stop;

Ditto.

> Source/WebCore/rendering/RenderView.cpp:873
> +        }

The code looks exactly the same than the above, would it be possible to refactor it in some function?
Comment 12 EFL EWS Bot 2013-08-09 08:00:24 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1353465
Comment 13 EFL EWS Bot 2013-08-09 08:28:31 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1414369
Comment 14 Early Warning System Bot 2013-08-09 08:41:43 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1399219
Comment 15 Early Warning System Bot 2013-08-09 08:42:53 PDT 
Comment on attachment 208429 [details]
Patch

Attachment 208429 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1350447
Comment 16 Javier Fernandez 2013-08-09 12:07:19 PDT 
Created attachment 208447 [details]
Patch
Comment 17 Dave Hyatt 2013-08-14 13:34:42 PDT 
Comment on attachment 208447 [details]
Patch

Patch looks fine, but you need a test.
Comment 18 Javier Fernandez 2013-08-14 15:30:32 PDT 
Created attachment 208765 [details]
Patch

Added missing layout tests
Comment 19 Dave Hyatt 2013-09-04 12:26:34 PDT 
Comment on attachment 208765 [details]
Patch

r=me
Comment 20 WebKit Commit Bot 2013-09-04 12:51:25 PDT 
Comment on attachment 208765 [details]
Patch

Clearing flags on attachment: 208765

Committed r155058: <http://trac.webkit.org/changeset/155058>
Comment 21 WebKit Commit Bot 2013-09-04 12:51:29 PDT 
All reviewed patches have been landed.  Closing bug.