Bug 13675 - Focus rings don't repaint properly on editable continuations
Summary: Focus rings don't repaint properly on editable continuations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-11 01:48 PDT by Dave Hyatt
Modified: 2007-05-11 02:58 PDT (History)
0 users

See Also:


Attachments
Demo of the problem... just start typing and hitting enter. (91 bytes, text/html)
2007-05-11 01:48 PDT, Dave Hyatt
no flags Details
Patch to improve painting (8.80 KB, patch)
2007-05-11 01:51 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch with layout test (110.10 KB, patch)
2007-05-11 02:05 PDT, Dave Hyatt
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2007-05-11 01:48:03 PDT
Reduction forthcoming.
Comment 1 Dave Hyatt 2007-05-11 01:48:35 PDT
Created attachment 14483 [details]
Demo of the problem... just start typing and hitting enter.
Comment 2 Dave Hyatt 2007-05-11 01:51:06 PDT
Created attachment 14484 [details]
Patch to improve painting

This patch makes continuation painting be done by the containing block of the entire continuation run.  If any component of the continuation needs to paint an outline, it will schedule the paint with the enclosing block who will then paint it.

This patch preserves the old paint path when a layer is present.

I also patched absoluteOutlineBox and absoluteClippedOverflowRect so that blocks in the continuation will assume they have outlines if the inline that generated them does.
Comment 3 Dave Hyatt 2007-05-11 02:05:28 PDT
Created attachment 14485 [details]
Patch with layout test

Added a layer paint test.  I will also turn the attachment into an editing layout test.
Comment 4 mitz 2007-05-11 02:17:55 PDT
+<p><span>Content before<h3>Middle</h3>No content after.</span></p>

"No content after." is... content ;-)
Comment 5 Dave Hyatt 2007-05-11 02:24:44 PDT
Details.... :)
Comment 6 Oliver Hunt 2007-05-11 02:27:14 PDT
Comment on attachment 14485 [details]
Patch with layout test

+    if (table->isEmpty())
+        return;
+        
+    RenderFlowSequencedSet* continuations = table->get(this);
+    if (!continuations)
+        return;

is the isEmpty() check necessary given we also check the result of get?

Looks good, i'd like you to run the pixel tests though, and (you'll hate me for this) manually check any pixel tests that fail
Comment 7 Dave Hyatt 2007-05-11 02:58:27 PDT
Fixed in r21397.