Re: [webkit-dev] Adding reverse flag to Widget::SetFocus?

[Fady Samuel]

Hi Youifumi,

I'm a little bit confused about what you did. It seems like youalways
select the first subelement in your elemnt.

void DateTimeEditElement::focusByOwner()
{
if (DateTimeFieldElement* field = fieldAt(0))
field-focus();
}

I don't understand how Shift+Tab works in your code?

Thanks,

Fady

On Mon, Sep 10, 2012 at 9:39 PM, Yoshifumi Inoue yo...@google.com wrote:

 Hi Fady,

 I have same experience about focus in internal element within container
 element.  I tried to do same thing for multiple fields time input UI for
 input type=time. In the implementation, fields are implemented in shadow
 DOM hosted by input element.
 We call fields are DateTimeFieldElement and container for them as
 DateTimeEditElement.

 On first implementation, DateTimeEditElement controlled focus. When input
 element focused, DateTimeEditElement sets the first field. It worked fine
 except for Shift+Tab.
 http://trac.webkit.org/changeset/125263

 I studied FocusController and EventHandler::defaultTabEventHandler.

 http://trac.webkit.org/browser/trunk/Source/WebCore/page/FocusController.cpp

 http://trac.webkit.org/browser/trunk/Source/WebCore/page/EventHandler.cpp

 I thought two options:
 1. Factor out defaultTabEventHandler.
   = FocusController::advanceFocus requires keyboard event object for
 Option+Tab on Mac.
 2. Create fake-keyboard event containing Shift+Tab then pass to
 defaultEventHandler.

 It seems making fake-keyboard event is little bit ugly.

 So, our conclusion is making DateTimeEditElement not to manage focus ==
 FocusController manages focus.
 http://trac.webkit.org/changeset/127226

 As of this experiment,  setFocus() with direction parameter doesn't solve
 controlling focus by container. Container wants to know direction of focus
 navigation from event object.

 The pseudo code can be written as:

 TimeInputElement::defaultEventHandler(Event* event)
 {
 if (event-isFocusEvent()) {
 if (event-focuseDirection() == FocusDiectionForward)
   setFocusToField(0);
else
  setFocusToField(m_lastFieldIndex);

// Not sure about FocusDirectionUp/Down
// Need RTL support for FocusDirectionLeft/Right. Move focus
 Left/Right on physical location.
 }
 }




 On Tue, Sep 11, 2012 at 4:32 AM, Fady Samuel fsam...@chromium.org wrote:

 Hi all,

 I'm working on a plugin that needs to deal with focus. I'd like this
 plugin to look an feel just like any other DOM element including the way it
 deals with focus. The problem that I have is Widgets know about focus but
 not focus direction (forward/reverse). My plugin has several logical
 subelements. The initial subelement to select is based on the direction
 taken to arrive to the plugin by keyboard  (Tab v.s. Shift-Tab).

 I'd like to add a new parameter with a flag or enum three states perhaps:
 Foward Reverse, Neither to Widget::SetFocus.

 This is going to require many one-line changes throughout a number of
 files throughout WebCore.

 Before I embark on such a change, I'd like to get some feedback at this
 point. Is this a reasonable thing to add? Does anyone oppose this change?
 Is there a cleaner way to add this support?

 Thanks,

 Fady

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Adding reverse flag to Widget::SetFocus?

[Fady Samuel]

Hi all,

I'm working on a plugin that needs to deal with focus. I'd like this plugin
to look an feel just like any other DOM element including the way it deals
with focus. The problem that I have is Widgets know about focus but not
focus direction (forward/reverse). My plugin has several logical
subelements. The initial subelement to select is based on the direction
taken to arrive to the plugin by keyboard  (Tab v.s. Shift-Tab).

I'd like to add a new parameter with a flag or enum three states perhaps:
Foward Reverse, Neither to Widget::SetFocus.

This is going to require many one-line changes throughout a number of files
throughout WebCore.

Before I embark on such a change, I'd like to get some feedback at this
point. Is this a reasonable thing to add? Does anyone oppose this change?
Is there a cleaner way to add this support?

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Tab focus order for plugins

[Fady Samuel]

Hi all,

ap@ is away until July 18, and I was hoping to get some feedback on this
change for the WebCore changes.

In a nutshell, we'd like certain plugins in chromium to participate in tab
focus. My understanding from a recent thread in
whatwghttp://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-June/036460.html
is
that which controls participate in the tab focus cycle is up to the UA.

This change plumbs this decision out of WebCore to allow the WebKit
embedder to decide whether or not to allow a plugin to participate in tab
order on a per-plugin basis.

Are there any thoughts or objections to this patch?

By default, nothing changes. However certain plugins can choose to
participate in the tab focus order, if they support it.

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Tab focus order for plugins

[Fady Samuel]

Oops, lost the link in the email:
https://bugs.webkit.org/show_bug.cgi?id=88958

On Tue, Jun 26, 2012 at 1:26 PM, Fady Samuel fsam...@chromium.org wrote:

 Hi all,

 ap@ is away until July 18, and I was hoping to get some feedback on this
 change for the WebCore changes.

 In a nutshell, we'd like certain plugins in chromium to participate in tab
 focus. My understanding from a recent thread in 
 whatwghttp://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-June/036460.html
  is
 that which controls participate in the tab focus cycle is up to the UA.

 This change plumbs this decision out of WebCore to allow the WebKit
 embedder to decide whether or not to allow a plugin to participate in tab
 order on a per-plugin basis.

 Are there any thoughts or objections to this patch?

 By default, nothing changes. However certain plugins can choose to
 participate in the tab focus order, if they support it.

 Thanks,

 Fady

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Understanding WebKit layering and layering violations

[Fady Samuel]

Hi Adam,

Your diagram is a start. I don't recall all my layering violation
confusions but the latest confusion I had was in regards to what goes in
the Source/WebCore/platform directory?

Why are things like ScrollView ScrollableArea, Timer, or even
NotImplemented or Length in that directory? A lot of these things don't
look like platform-specific code. They're also not necessarily interfaces
to platform-specific code. So what are they? How does one decide something
belongs in Source/WebCore/platform. What can things in this directory
depend on and what can they not depend on?

Thanks,

Fady

On Thu, Jan 5, 2012 at 7:39 PM, Adam Barth aba...@webkit.org wrote:

 On Thu, Jan 5, 2012 at 4:18 PM, Fady Samuel fsam...@chromium.org wrote:
  I've been working on WebKit off and on for a while now but I must admit
  that, up to this point, I still don't have a firm grasp of all the
 layering
  in WebKit. What depends on what, and what cannot depend on what? What is
 the
  motivation of each of these individual layers? I understand the need to
 have
  WebKit supported under multiple platforms and with any embedder, and but
 I
  often don't fully understand what constitues a layering violation or why.
 
  Could someone please summarize or provide a link that explains all this?
 I
  think this would be beneficial to the entire WebKit community!

 There's one diagram:


 https://docs.google.com/drawings/d/10WlCj2J3arxf4cDGRKteNinaP755iFnmYtYtnNSCQOY/edit?authkey=CP6plYAIhl=en_US

 It's not perfectly accurate because it's something of a proposal, but
 it should give you a high-level view.  Do you have specific questions?
  I'm happy to draw more pictures if that would be helpful.

 Adam

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Understanding WebKit layering and layering violations

[Fady Samuel]

Hi all,

I've been working on WebKit off and on for a while now but I must admit
that, up to this point, I still don't have a firm grasp of all the layering
in WebKit. What depends on what, and what cannot depend on what? What is
the motivation of each of these individual layers? I understand the need to
have WebKit supported under multiple platforms and with any embedder, and
but I often don't fully understand what constitues a layering violation or
why.

Could someone please summarize or provide a link that explains all this? I
think this would be beneficial to the entire WebKit community!

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Confusion over over-constrained case in RenderBox::computePositionedLogicalWidthUsing

[Fady Samuel]

Hi all,

I'm currently studying layout code in RenderBox very carefully as I work on
this:
https://bugs.webkit.org/show_bug.cgi?id=47738

I'm a bit confused about something fairly subtle. In the overconstrained
case where logical left, logical width, and logical right are all
specified, we should ignore right (in ltr and left in rtl), correct?

First a clarification please: is the logical left actually the right side
of the box in rtl mode?
Secondly, I'm confused about the availableSpace calculation (at line 2706
in my local branch):

const LayoutUnit availableSpace = containerLogicalWidth - (logicalLeftValue
 + logicalWidthValue + logicalRight.calcValue(containerLogicalWidth) +
 bordersPlusPadding);


Why is logicalRight.calcValue(containerLogicalWidth) in that computation at
all? It seems to me that the available space for left/right margin (in ltr)
is where the box is positioned relative to the left edge of the container
box plus its content width + bordersPlusPadding. I can't figure out how
logicalRight plays a role here. Could someone please clarify?
In fact, a couple of lines above I see the following comment:

// NOTE: It is not necessary to solve for 'right' in the over constrained
 // case because the value is not used for any further calculations.


This seems to contradict the availableSpace line. Is this a stale comment
or is this a bug, or am I just misunderstanding something?

Thanks,
Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Forcing relayout on style change

[Fady Samuel]

Hi all,

I've been in the process of implementing the aspect-ratio CSS property. I
have a patch that plumbs the property to RenderStyle. I'd like to include
code that force relayout upon a change to aspect ratio on the associated
RenderBox..

Here's a patch from a couple of days ago of what I currently have (modulo
some bug fixes). https://bugs.webkit.org/show_bug.cgi?id=72350

Could someone please point me to a place in the code to do this?

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Forcing relayout on style change

[Fady Samuel]

Perfect! This is exactly what I was looking for, thanks!

Fady

On Thu, Nov 17, 2011 at 11:11 AM, Dan Bernstein m...@apple.com wrote:



 On Nov 17, 2011, at 7:55 AM, Fady Samuel fsam...@chromium.org wrote:

 Hi all,

 I've been in the process of implementing the aspect-ratio CSS property. I
 have a patch that plumbs the property to RenderStyle. I'd like to include
 code that force relayout upon a change to aspect ratio on the associated
 RenderBox..

 Here's a patch from a couple of days ago of what I currently have (modulo
 some bug fixes). https://bugs.webkit.org/show_bug.cgi?id=72350

 Could someone please point me to a place in the code to do this?


 RenderStyle::diff()

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] CSS Aspect Ratio Parsing Stage Up For Review

[Fady Samuel]

Hi Edward,

Object fit doesn't solve quite the same problem. This property is meant to
work with all kinds of boxes, except table cells.

Fady

On Tue, Oct 25, 2011 at 2:23 PM, Edward O'Connor eocon...@apple.com wrote:

 Hi Fady,

 You wrote:
  I've uploaded the parsing stage for the -webkit-aspect-ratio property
  that was proposed here:
 
  http://www.xanthir.com/blog/b4810
 […]
  I was wondering if anyone would be interested in reviewing this and
  getting this incorporated into WebKit?

 I think we should wait until the aspect-ratio property is incorporated
 into an Editor's Draft in the CSS WG before exposing an implementation
 of it to Web content. Also, the CSS Image Values and Replaced Content
 module already has an object-fit property[1] which WebKit doesn't yet
 implement. We should probably make a pass at that before having a go at
 something similar that's not even in an ED.


 Ted

 1. http://dev.w3.org/csswg/css3-images/#object-fit
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] CSS Aspect Ratio Parsing Stage Up For Review

[Fady Samuel]

Hi all,

I've uploaded the parsing stage for the -webkit-aspect-ratio property that
was proposed here:

http://www.xanthir.com/blog/b4810

The parsing implementation can be found here:

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

An old, experimental implementation of the entire feature was uploaded last
year:

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

and discussed here:

http://lists.w3.org/Archives/Public/www-style/2010Oct/0046.html

http://lists.w3.org/Archives/Public/www-style/2011Oct/0550.html

I was wondering if anyone would be interested in reviewing this and getting
this incorporated into WebKit?

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Compiling chromium webkit unit tests on Mac

[Fady Samuel]

Hi all,

I apologize for sending this out to such a broad audience, but I'd like to
know how to compile the chromium webkit unit tests on Mac? I tried this:

xcodebuild -project
third_party/WebKit/Source/WebKit/chromium/WebKit.xcodeproj -configuration
Debug -target webkit_unit_tests

It complains about a missing libskia. How do I build skia then? Or is there
a better way to build the unit tests?

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Making PopupMenuClient testable

[Fady Samuel]

*ping*

I'd love to get some feedback on
https://bugs.webkit.org/show_bug.cgi?id=69631. :-)

Thanks,

Fady

On Mon, Oct 10, 2011 at 5:27 PM, Fady Samuel fsam...@chromium.org wrote:

 Please review then? :-)

 Fady

 On Mon, Oct 10, 2011 at 9:11 PM, Darin Adler da...@apple.com wrote:
  On Oct 10, 2011, at 7:57 AM, Fady Samuel wrote:
 
  My solution in 69631 is to expose the bounding box rect through the
 PopupMenuClient interface and make it testable through window.internals.
 I've also included a layout test to show how one would test this. Right now
 the test fails, but once 66062 is applied the test will pass.
 
  Is this a reasonable approach?
 
  This seems OK to me.
 
 -- Darin
 
 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Making PopupMenuClient testable

[Fady Samuel]

+darin

Johnny, I'm not sure what you mean. By the render dump, do you mean the
pixel dump? Popups don't show up in pixel dumps. I'm not sure what
information you can get from that.

The bug that I'm trying to address, is, among other things, the bounding box
rect of the PopupMenuClient that is passed into the platform layer to show
the popup does not take transforms into account. My solution in 69631 is to
expose the bounding box rect through the PopupMenuClient interface and make
it testable through window.internals. I've also included a layout test to
show how one would test this. Right now the test fails, but once 66062 is
applied the test will pass.

Is this a reasonable approach?

Thanks,

Fady

On Mon, Oct 10, 2011 at 10:41 AM, Johnny Ding j...@google.com wrote:



 在 2011年10月7日 下午11:56，Fady Samuel fsam...@chromium.org写道：

 Hi all,

 I've just uploaded a patch https://bugs.webkit.org/show_bug.cgi?id=69631 
 that's
 intended to make PopupMenuClient testable.

 On certain Chromium platforms, the position of the select element's
 popup is incorrect if the page's scale factor is not 1.0.

 The fix to the problem is simple, see
 https://bugs.webkit.org/show_bug.cgi?id=66062 , but since popups are
 external to WebKit on most platforms, this is difficult to test.

 The patch above in bug 69631 attempts to addresses this, albeit in a less
 than ideal way. I'm not sure what's the best way to test this, but this is
 my stab at it.

 Even if  the popup is external, the render dump (not text dump) still
 contains layout information. You can write a layout test with page's scale
 factor not 1.0, then generate the expectation results among WebKit ports.
  Do you think Is that enough for your test?


 I'd really appreciate any comments or suggestions you can provide on my
 patch for bug 69631.

 Thanks,

 Fady

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev




 --
 Best regards,
 Johnny(Jianning) Ding | Software Engineer | j...@google.com |
  (86)189-1186-0082


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Making PopupMenuClient testable

[Fady Samuel]

Hi Johnny,

 What I mean is regular render tree dump, not pixel dump, not text dump. 
 Please refer here for test expectation and see in TestShell::dump in DRT 
 chromium port

 I don't believe a render tree dump will solve this problem because
the issue is the rect that is being passed into the platform layer
doesn't take transforms into account.

 After landing https://bugs.webkit.org/show_bug.cgi?id=66062, will 
 the PopupMenuClient take transforms into account?

Well, in particular, RenderMenuList will pass the appropriate
transformed rect of the PopupMenuCliient to the popup.

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Adding Scroll Padding to allow scroll beyond the edge of the page (within some bounds)

[Fady Samuel]

The way I currently have it implemented, scroll padding affects the
maximumScrollPosition and the minimumScrollPosition.

Those two methods are virtual. If scroll padding was placed in FrameView,
then it could also override maximumScrollPosition and minimumScrollPosition
to take padding into account.

I believe it's reasonable to do this in FrameView, but I'm open to
suggestions.

The one issue is ensuring that the scroll bounds are always checked through
maximumScrollPosition and minimumScrollPosition rather than scrollSize
(which is determined by the content only).

Fady

On Fri, Oct 7, 2011 at 11:27 AM, Robert Kroeger rjkro...@chromium.orgwrote:

 On Thu, Oct 6, 2011 at 2:02 PM, Antonio Gomes toniki...@gmail.com wrote:
  From what I tested on iOS5 Mobo Safari, it also overscroll's the
  overflow:scroll case (generally div's).
 
  On Thu, Oct 6, 2011 at 1:47 PM, Fady Samuel fsam...@chromium.org
 wrote:
 
  Hi Anders,
  At this point in time, we see no reason to allow for non-zero padding in
  overflow:scroll regions because you can always just move the page into a
  unobstructed area and then scroll through that overflow region.

 I agree with Fady that the feature he describes only requires scroll
 padding on the page. But would it be better from a code structure
 viewpoint to actually add this functionality to ScrollableArea?

 Rob.

  Thanks,
  Fady
 
  On Thu, Oct 6, 2011 at 1:43 PM, Anders Carlsson ander...@apple.com
  wrote:
 
  Do you envision this being useful on overflow:scroll regions as well or
  is it just frames? If it's just frames, then it seems like something we
  could keep in ScrollView? (I haven't looked at the patch yet).
  - Anders
  On Oct 6, 2011, at 10:41 AM, Fady Samuel wrote:
 
  Hi Anders,
  Thanks for your reply.
  Yes, you are correct. This padding would be between the content and the
  overhang area.
  Thanks,
  Fady
  On Thu, Oct 6, 2011 at 1:32 PM, Anders Carlsson ander...@apple.com
  wrote:
 
  Hi Fady,
  so if I'm understanding correctly, in the context of rubber-band
  scrolling, this padding would be between the content and the overhang
 area?
  As far as constrainsScrollingToContentEdge goes, I'd like to get rid
 of
  it and just have two scroll functions, one that constrains to the
 content
  edge and one that doesn't.
  - Anders
  On Oct 6, 2011, at 10:03 AM, Fady Samuel wrote:
 
  Hi all,
 
  We'd like to provide a general mechanism in WebKit for embedders to
  scroll page content so that it is not hidden by embedder-provided UI
  elements that overlap the page.
 
  In some cases, if a floating UI element overlaps the edge of the page,
  we'd like to allow the embedder to scroll beyond the edge of the page
 to
  allow the hidden content to move to an area that isn't overlapped by
 UI
  elements. This feature is orthogonal to rubber band scrolling.
 
  One approach we considered taking is to allow the platform to set
  scroll padding to a FrameView/ScrollableArea to allow scrolling
 beyond the
  edge of the page.
  As a more concrete example, one can imagine a persistent Chromium
  extension that floats above the edge of the page. A link may lie
 behind the
  floating window.  That link would be inaccessible unless the page is
 allowed
  to scroll beyond its edge.
 
  An experimental and incomplete implementation of this idea can be
 found
  here: https://bugs.webkit.org/show_bug.cgi?id=68184
 
  After some additional consideration since this patch was posted, I
 don't
  believe scroll padding should interact with
  ScrollView::constrainsScrollingToContentEdge the way it does in the
 patch.
  Instead, I feel that scroll padding should be ignored
  if constrainsScrollingToContentEdge is false. That way rubber band
 scrolling
  is not affected at all by this.
 
  What are your thoughts and suggestions? Is this feature sufficiently
  general to be implemented in WebCore? What are your thoughts about its
  interaction with ScrollView::constrainsScrollingToContentEdge?
 
  Thanks,
  Fady
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 
 
 
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 
 
 
  --
  --Antonio Gomes
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Making PopupMenuClient testable

[Fady Samuel]

Hi all,

I've just uploaded a patch https://bugs.webkit.org/show_bug.cgi?id=69631 that's
intended to make PopupMenuClient testable.

On certain Chromium platforms, the position of the select element's popup
is incorrect if the page's scale factor is not 1.0.

The fix to the problem is simple, see
https://bugs.webkit.org/show_bug.cgi?id=66062 , but since popups are
external to WebKit on most platforms, this is difficult to test.

The patch above in bug 69631 attempts to addresses this, albeit in a less
than ideal way. I'm not sure what's the best way to test this, but this is
my stab at it.

I'd really appreciate any comments or suggestions you can provide on my
patch for bug 69631.

Thanks,

Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Adding Scroll Padding to allow scroll beyond the edge of the page (within some bounds)

[Fady Samuel]

Hi all,

We'd like to provide a general mechanism in WebKit for embedders to scroll
page content so that it is not hidden by embedder-provided UI elements that
overlap the page.

In some cases, if a floating UI element overlaps the edge of the page, we'd
like to allow the embedder to scroll beyond the edge of the page to allow
the hidden content to move to an area that isn't overlapped by UI
elements. This feature is orthogonal to rubber band scrolling.

One approach we considered taking is to allow the platform to set scroll
padding to a FrameView/ScrollableArea to allow scrolling beyond the edge of
the page.

As a more concrete example, one can imagine a persistent Chromium extension
that floats above the edge of the page. A link may lie behind the floating
window.  That link would be inaccessible unless the page is allowed to
scroll beyond its edge.

An experimental and incomplete implementation of this idea can be found
here: https://bugs.webkit.org/show_bug.cgi?id=68184

After some additional consideration since this patch was posted, I don't
believe scroll padding should interact with
ScrollView::constrainsScrollingToContentEdge the way it does in the patch.
Instead, I feel that scroll padding should be ignored
if constrainsScrollingToContentEdge is false. That way rubber band scrolling
is not affected at all by this.

What are your thoughts and suggestions? Is this feature sufficiently general
to be implemented in WebCore? What are your thoughts about its interaction
with ScrollView::constrainsScrollingToContentEdge?

Thanks,
Fady
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Adding Scroll Padding to allow scroll beyond the edge of the page (within some bounds)

[Fady Samuel]

Hi Anders,

Thanks for your reply.

Yes, you are correct. This padding would be between the content and the
overhang area.

Thanks,

Fady

On Thu, Oct 6, 2011 at 1:32 PM, Anders Carlsson ander...@apple.com wrote:

 Hi Fady,

 so if I'm understanding correctly, in the context of rubber-band scrolling,
 this padding would be between the content and the overhang area?

 As far as constrainsScrollingToContentEdge goes, I'd like to get rid of it
 and just have two scroll functions, one that constrains to the content edge
 and one that doesn't.

 - Anders

 On Oct 6, 2011, at 10:03 AM, Fady Samuel wrote:

 Hi all,

 We'd like to provide a general mechanism in WebKit for embedders to scroll
 page content so that it is not hidden by embedder-provided UI elements that
 overlap the page.

 In some cases, if a floating UI element overlaps the edge of the page, we'd
 like to allow the embedder to scroll beyond the edge of the page to allow
 the hidden content to move to an area that isn't overlapped by UI
 elements. This feature is orthogonal to rubber band scrolling.

 One approach we considered taking is to allow the platform to set scroll
 padding to a FrameView/ScrollableArea to allow scrolling beyond the edge of
 the page.

 As a more concrete example, one can imagine a persistent Chromium extension
 that floats above the edge of the page. A link may lie behind the floating
 window.  That link would be inaccessible unless the page is allowed to
 scroll beyond its edge.

 An experimental and incomplete implementation of this idea can be found
 here: https://bugs.webkit.org/show_bug.cgi?id=68184

 After some additional consideration since this patch was posted, I don't
 believe scroll padding should interact with
 ScrollView::constrainsScrollingToContentEdge the way it does in the patch.
 Instead, I feel that scroll padding should be ignored
 if constrainsScrollingToContentEdge is false. That way rubber band scrolling
 is not affected at all by this.

 What are your thoughts and suggestions? Is this feature sufficiently
 general to be implemented in WebCore? What are your thoughts about its
 interaction with ScrollView::constrainsScrollingToContentEdge?

 Thanks,
 Fady
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Adding Scroll Padding to allow scroll beyond the edge of the page (within some bounds)

[Fady Samuel]

Hi Anders,

At this point in time, we see no reason to allow for non-zero padding in
overflow:scroll regions because you can always just move the page into a
unobstructed area and then scroll through that overflow region.

Thanks,

Fady

On Thu, Oct 6, 2011 at 1:43 PM, Anders Carlsson ander...@apple.com wrote:

 Do you envision this being useful on overflow:scroll regions as well or is
 it just frames? If it's just frames, then it seems like something we could
 keep in ScrollView? (I haven't looked at the patch yet).

 - Anders

 On Oct 6, 2011, at 10:41 AM, Fady Samuel wrote:

 Hi Anders,

 Thanks for your reply.

 Yes, you are correct. This padding would be between the content and the
 overhang area.

 Thanks,

 Fady

 On Thu, Oct 6, 2011 at 1:32 PM, Anders Carlsson ander...@apple.comwrote:

 Hi Fady,

 so if I'm understanding correctly, in the context of rubber-band
 scrolling, this padding would be between the content and the overhang area?

 As far as constrainsScrollingToContentEdge goes, I'd like to get rid of it
 and just have two scroll functions, one that constrains to the content edge
 and one that doesn't.

 - Anders

 On Oct 6, 2011, at 10:03 AM, Fady Samuel wrote:

 Hi all,

 We'd like to provide a general mechanism in WebKit for embedders to scroll
 page content so that it is not hidden by embedder-provided UI elements that
 overlap the page.

 In some cases, if a floating UI element overlaps the edge of the page,
 we'd like to allow the embedder to scroll beyond the edge of the page to
 allow the hidden content to move to an area that isn't overlapped by UI
 elements. This feature is orthogonal to rubber band scrolling.

 One approach we considered taking is to allow the platform to set scroll
 padding to a FrameView/ScrollableArea to allow scrolling beyond the edge of
 the page.

 As a more concrete example, one can imagine a persistent Chromium
 extension that floats above the edge of the page. A link may lie behind the
 floating window.  That link would be inaccessible unless the page is allowed
 to scroll beyond its edge.

 An experimental and incomplete implementation of this idea can be found
 here: https://bugs.webkit.org/show_bug.cgi?id=68184

 After some additional consideration since this patch was posted, I don't
 believe scroll padding should interact with
 ScrollView::constrainsScrollingToContentEdge the way it does in the patch.
 Instead, I feel that scroll padding should be ignored
 if constrainsScrollingToContentEdge is false. That way rubber band scrolling
 is not affected at all by this.

 What are your thoughts and suggestions? Is this feature sufficiently
 general to be implemented in WebCore? What are your thoughts about its
 interaction with ScrollView::constrainsScrollingToContentEdge?

 Thanks,
 Fady
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev





___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Making top-level style changes through window.internals

[Fady Samuel]

Hi all,

pageScaleFactor is a document level CSS scaling style. Often times, we'd
like to be able to apply style at the document level when writing layout
tests. As far as I'm aware, there's no way to do this in javascript in a
layout test? Is this correct? If so, would anyone object to exposing
document-level styles to window.internals? If not there, is there anywhere
else where this can be exposed for testing purposes?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] WebKit layout rect, and scroll area

[Fady Samuel]

Hi all,

I was wondering if it's possible to tell webkit to layout a page at a
particular resolution while telling it to provide a smaller scroll area?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Six new layout tests missing expected results

[Fady Samuel]

So sorry. I had to leave before I got in another patch that fixed the
problem. Will have it fixed tomorrow.

Fady

On Wed, Aug 18, 2010 at 11:08 PM, Dimitri Glazkov dglaz...@chromium.orgwrote:

 Yep. It's his. He doesn't have a Mac, so he's been trying to collect
 results from the bot pretty much all day today. Given that he's not a
 committer either, that's twice as hard :)

 :DG

 On Wed, Aug 18, 2010 at 7:37 PM, David Levin le...@chromium.org wrote:
  Fady, it looks
  (
 http://trac.webkit.org/search?q=tables/hittesting/filltable-emptycells.html
 )
  like this is due to your patch.
  dave
 
  On Wed, Aug 18, 2010 at 7:32 PM, Maciej Stachowiak m...@apple.com
 wrote:
 
  Who knows what is up with these? I'm guessing they came from the same
  patch. Please fix if you know what these are about.
 
  fast/table/simple_paint.html - new
  tables/hittesting/filltable-emptycells.html - new
  tables/hittesting/filltable-levels.html - new
  tables/hittesting/filltable-outline.html - new
  tables/hittesting/filltable-rtl.html - new
  tables/hittesting/filltable-stress.html - new
 
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
 
 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Some landed patches have incorrect date in commit messages and ChangeLog

[Fady Samuel]

What about the case where between the time you submit the patch and it gets
committed, a day passes by...does the commit queue fix this automatically,
or does someone have to go back and fix it manually?

Fady

On Mon, Aug 9, 2010 at 1:51 PM, Jian Li jia...@chromium.org wrote:

 Hi,

 I noticed that several patches landed recently have the incorrect date put
 in the commit messages and ChangeLog. Please fix the incorrect information
 in ChangeLog. For the commit messages, anyone know if we can fix them or
 not?

 Please make sure the correct date is used when you land your patch
 manually. Thanks.

 Jian


 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] SVG cloneNode?

[Fady Samuel]

I have just added a bugzilla bug:

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

I looked at SVGAnimatedProperty.h briefly but I can't think of a clever way
to modify it so that there's a nice, general way to copy these animated
attributes to the cloned node. I believe a bunch of files will need to be
modified but I could be wrong.

Fady

On Wed, Jun 30, 2010 at 1:53 PM, Darin Adler da...@apple.com wrote:

 As I see it, the issue shown in your test case is that cloning animated
 attributes does not clone the animation parameters. That’s something we will
 need to solve. We should not solve it by overriding
 copyNonAttributeProperties and hand-writing it in each class.

 I don’t know enough about the SVG animated attributes support to know how
 to solve the problem myself, but you should make sure we have a bug report
 in bugs.webkit.org mentioning cloneNode and SVG animated attributes so we
 can discuss there how to fix it.

 I imagine the code changes will be in SVGElement.h/cpp and
 SVGAnimatedProperty.h/cpp.

-- Darin


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Integral Type Size of pointer for platform?

[Fady Samuel]

Does WebKit define somewhere an integral type that's guaranteed to be the
same size as the platform's pointer type?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Should Table Cell (TD) children inherit height?

[Fady Samuel]

Yup, I just wanted to rule this out. Then this means that the div's height
is auto, correct? That means it sizes itself to fit its contents. Now,
consider a similar situation, suppose its contents have a percent instead,
say:

  div id=div1 style=background-color: red; height:100%;
  div id=div2 style=background-color: blue;
div id=div3 style=background-color: green; height:100%/div
  /div
  /div

Both Firefox and WebKit show the same behavior. The resulting color is
green. Why?  Why isn't it red? How does the height inheritance work here?

Thanks,

Fady



On Mon, Jun 28, 2010 at 10:11 PM, Maciej Stachowiak m...@apple.com wrote:


 On Jun 28, 2010, at 6:32 PM, David Kilzer wrote:

  How do Firefox and MSIE render the test case?

 Our rendering on this content matches Firefox in both quirks and standards
 mode. I also do not know if a reason per spec for the div to get 100%
 height. Height does not inherit.

  - Maciej

 
  Dave
  --
  Sent from my iPhone 3GS
 
  On Jun 28, 2010, at 11:42 AM, Fady Samuel fsam...@chromium.org wrote:
 
  My understanding is that in quirks mode, table height=100% causes
 the table to fill the entire client window. Now, I believe the height
 attribute should trickle down to the TR and TD tags, correct? What about
 children of a TD tag? If you add an empty div child to a table, should it
 fit the whole cell, or should it have a size of 0?
 
 
  table width=100% border=0 style=height:100%;
 tr
 td style=background-color: red;
   div id=drawing style=background-color: green;/div
 /td
 /tr
  /table
 
 
  WebKit will show you red instead of green because the div does not have
 a height of 100%.
 
  Now if you make the div's height 100%, you get green.
 
  Should a cell's child inherit height?
 
  Thanks,
 
  Fady Samuel
 
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Should Table Cell (TD) children inherit height?

[Fady Samuel]

My understanding is that in quirks mode, table height=100% causes the
table to fill the entire client window. Now, I believe the height attribute
should trickle down to the TR and TD tags, correct? What about children of a
TD tag? If you add an empty div child to a table, should it fit the whole
cell, or should it have a size of 0?


table width=100% border=0 style=height:100%;
tr
td style=background-color: red;
  div id=drawing style=background-color: green;/div
/td
/tr
/table


WebKit will show you red instead of green because the div does not have a
height of 100%.

Now if you make the div's height 100%, you get green.

Should a cell's child inherit height?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

So a first patch is now available for review here:

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

Thanks, https://bugs.webkit.org/show_bug.cgi?id=40775

Fady

2010/6/8 Fady Samuel fsam...@google.com

 So I was just about ready to post a potential patch for keeping track of
 all cells that lie on a given grid slot. It seems that information is
 insufficient for correct rendering.

 Consider the attached test case:

 1 R1C4 spans 4 rows, and should be drawn first.
 2. R2C1 spans 4 columns and overlaps R1C4 (lies above it)
 3. R4C1 spans 4 columsn and overlaps R1C4 (lies above it)

 Consider the case where the paint rect is the lower right grid slot.
 Knowing that R4C1 and R1C4 overlap, we draw R1C4 first and then R4C1 (in
 that order as they both lie inside the single slot dirty rect). Now we've
 drawn R1C4 ABOVE R2C1 This results in a rendering artifact. I'm
 currently working a solution that places cells in different layers and if a
 cell is dirty, we redraw it and all layers above it. How a layer is defined
 and formed and the data structures involved to do this efficiently will be
 discussed here shortly.

 Fady


 On Wed, Jun 2, 2010 at 4:40 PM, Fady Samuel fsam...@google.com wrote:

 So I have completed all the following things locally:


 * First fix the grid to record all the cells at a given position.  This
 fixes a repaint bug.
 * Then change the hit-testing to work just like painting.  This makes hit
 testing more efficient and ensures hit testing and painting agree.
 * Then optimize hit-testing to binary-search if there are no overflowing
 cells.  (And optimize painting similarly)
 * Fix a table layout bug
 * created a bunch of additional simple table layout tests

 I did all the pieces locally first to make sure I understand how all the
 parts of table rendering fit together. However, I intend to get the pieces
 of this work out in in smaller patches to simplify the review process for
 reviewers.

 While I still have some more work to do testing, and splitting my work up
 into the suggested pieces, please expect bug reports and proposed patches to
 begin appearing shortly (within the next couple of days or so).

 Thanks,

 Fady Samuel


 On Wed, Jun 2, 2010 at 7:34 AM, Fady Samuel fsam...@google.com wrote:

 Ohh, I see, thanks Roland.

 Fady


 On Wed, Jun 2, 2010 at 3:25 AM, Roland Steiner rolandstei...@google.com
  wrote:

 AFAICT you could have an arbitrary number up to the width or height of
 the table, whichever is smaller, by combining row- and colspans - e.g. with
 3 ([v]aligns just to emphasize the overlapping):

 table border=1tbody
 trtdR1C1/tdtdR1C2/tdtd rowspan=3
 valign=bottom\\/td/tr
 trtdR2C1/tdtd rowspan=2 colspan=2 align=right
 valign=bottom///td/tr
 trtd colspan=3 align=right/td/tr
 /tbody/table

 - Roland

 On Wed, Jun 2, 2010 at 8:58 AM, Fady Samuel fsam...@google.com wrote:

 Hi David,

 Just so I'm certain, there's no way for more than two cells to overlap
 in a single grid slot, is there?

 Thanks,

 Fady Samuel


 On Thu, May 20, 2010 at 4:43 PM, David Hyatt hy...@apple.com wrote:

 On May 20, 2010, at 3:38 PM, Fady Samuel wrote:

  So what are the rules for stacking here? do the cells stack in the
 order in which they appear in the HTML?
 

 Correct, although note that backgrounds paint behind foregrounds, and
 hit testing works the same way, so it's not as simple as saying that 
 cell 7
 is always above cell 5.  They're interleaved, so from bottom to top:

 Cell 5 background
 Cell 7 background
 Cell 5 foreground (the number 5 in this example)
 Cell 7 foreground (the number 7 in this example)

 That's why if you hover over the actual number 5 in that example it
 will light up, but if you're in the background area overlap, the number 7
 will light up.

 http://www.w3.org/TR/CSS21/zindex.html

 Therefore you do have to know about all the cells at the position,
 since when you're in the foreground phase of hit testing, cell 7 will 
 say
 Nope, you're not inside my foreground, and so you then need to check 
 cell
 5, which will say Yes, you are inside my foreground.

 The grid structure just isn't expressive enough.  It needs to be
 patched to track all cells in a given row/column instead of just the 
 first
 one.

 dave
 (hy...@apple.com)



 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev






___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Initialization of Attribute Map

[Fady Samuel]

Hi all,

I'm looking at a bug listed on chromium.org (
http://code.google.com/p/chromium/issues/detail?id=46025) but it's actually
a WebKit bug...I've verified that the crash is in WebKit and Safari also
crashes on Mac. It seems that the javascript line:

ellipse.className.baseVal = cls1;

results in a call to StyledElement::classNames which then requests the
classNames from the attiribute map.

At the time, the attribute map does not exist. An assert fails because
it has not yet been created.

It seems a simple fix is to create it if it hasn't been created.

Looking at other code, it appears that the attribute map is created
lazily when needed.

Is this the solution or is there a fundamental assumption being
violated here that I should dig deeper into?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

Ohh, I see, thanks Roland.

Fady

On Wed, Jun 2, 2010 at 3:25 AM, Roland Steiner rolandstei...@google.comwrote:

 AFAICT you could have an arbitrary number up to the width or height of the
 table, whichever is smaller, by combining row- and colspans - e.g. with 3
 ([v]aligns just to emphasize the overlapping):

 table border=1tbody
 trtdR1C1/tdtdR1C2/tdtd rowspan=3 valign=bottom\\/td/tr
 trtdR2C1/tdtd rowspan=2 colspan=2 align=right
 valign=bottom///td/tr
 trtd colspan=3 align=right/td/tr
 /tbody/table

 - Roland

 On Wed, Jun 2, 2010 at 8:58 AM, Fady Samuel fsam...@google.com wrote:

 Hi David,

 Just so I'm certain, there's no way for more than two cells to overlap in
 a single grid slot, is there?

 Thanks,

 Fady Samuel


 On Thu, May 20, 2010 at 4:43 PM, David Hyatt hy...@apple.com wrote:

 On May 20, 2010, at 3:38 PM, Fady Samuel wrote:

  So what are the rules for stacking here? do the cells stack in the
 order in which they appear in the HTML?
 

 Correct, although note that backgrounds paint behind foregrounds, and hit
 testing works the same way, so it's not as simple as saying that cell 7 is
 always above cell 5.  They're interleaved, so from bottom to top:

 Cell 5 background
 Cell 7 background
 Cell 5 foreground (the number 5 in this example)
 Cell 7 foreground (the number 7 in this example)

 That's why if you hover over the actual number 5 in that example it will
 light up, but if you're in the background area overlap, the number 7 will
 light up.

 http://www.w3.org/TR/CSS21/zindex.html

 Therefore you do have to know about all the cells at the position, since
 when you're in the foreground phase of hit testing, cell 7 will say Nope,
 you're not inside my foreground, and so you then need to check cell 5,
 which will say Yes, you are inside my foreground.

 The grid structure just isn't expressive enough.  It needs to be patched
 to track all cells in a given row/column instead of just the first one.

 dave
 (hy...@apple.com)



 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

So I have completed all the following things locally:

* First fix the grid to record all the cells at a given position.  This
fixes a repaint bug.
* Then change the hit-testing to work just like painting.  This makes hit
testing more efficient and ensures hit testing and painting agree.
* Then optimize hit-testing to binary-search if there are no overflowing
cells.  (And optimize painting similarly)
* Fix a table layout bug
* created a bunch of additional simple table layout tests

I did all the pieces locally first to make sure I understand how all the
parts of table rendering fit together. However, I intend to get the pieces
of this work out in in smaller patches to simplify the review process for
reviewers.

While I still have some more work to do testing, and splitting my work up
into the suggested pieces, please expect bug reports and proposed patches to
begin appearing shortly (within the next couple of days or so).

Thanks,

Fady Samuel


On Wed, Jun 2, 2010 at 7:34 AM, Fady Samuel fsam...@google.com wrote:

 Ohh, I see, thanks Roland.

 Fady


 On Wed, Jun 2, 2010 at 3:25 AM, Roland Steiner 
 rolandstei...@google.comwrote:

 AFAICT you could have an arbitrary number up to the width or height of the
 table, whichever is smaller, by combining row- and colspans - e.g. with 3
 ([v]aligns just to emphasize the overlapping):

 table border=1tbody
 trtdR1C1/tdtdR1C2/tdtd rowspan=3 valign=bottom\\/td/tr
 trtdR2C1/tdtd rowspan=2 colspan=2 align=right
 valign=bottom///td/tr
 trtd colspan=3 align=right/td/tr
 /tbody/table

 - Roland

 On Wed, Jun 2, 2010 at 8:58 AM, Fady Samuel fsam...@google.com wrote:

 Hi David,

 Just so I'm certain, there's no way for more than two cells to overlap in
 a single grid slot, is there?

 Thanks,

 Fady Samuel


 On Thu, May 20, 2010 at 4:43 PM, David Hyatt hy...@apple.com wrote:

 On May 20, 2010, at 3:38 PM, Fady Samuel wrote:

  So what are the rules for stacking here? do the cells stack in the
 order in which they appear in the HTML?
 

 Correct, although note that backgrounds paint behind foregrounds, and
 hit testing works the same way, so it's not as simple as saying that cell 7
 is always above cell 5.  They're interleaved, so from bottom to top:

 Cell 5 background
 Cell 7 background
 Cell 5 foreground (the number 5 in this example)
 Cell 7 foreground (the number 7 in this example)

 That's why if you hover over the actual number 5 in that example it will
 light up, but if you're in the background area overlap, the number 7 will
 light up.

 http://www.w3.org/TR/CSS21/zindex.html

 Therefore you do have to know about all the cells at the position, since
 when you're in the foreground phase of hit testing, cell 7 will say 
 Nope,
 you're not inside my foreground, and so you then need to check cell 5,
 which will say Yes, you are inside my foreground.

 The grid structure just isn't expressive enough.  It needs to be patched
 to track all cells in a given row/column instead of just the first one.

 dave
 (hy...@apple.com)



 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev




___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

Hi David,

Just so I'm certain, there's no way for more than two cells to overlap in a
single grid slot, is there?

Thanks,

Fady Samuel

On Thu, May 20, 2010 at 4:43 PM, David Hyatt hy...@apple.com wrote:

 On May 20, 2010, at 3:38 PM, Fady Samuel wrote:

  So what are the rules for stacking here? do the cells stack in the order
 in which they appear in the HTML?
 

 Correct, although note that backgrounds paint behind foregrounds, and hit
 testing works the same way, so it's not as simple as saying that cell 7 is
 always above cell 5.  They're interleaved, so from bottom to top:

 Cell 5 background
 Cell 7 background
 Cell 5 foreground (the number 5 in this example)
 Cell 7 foreground (the number 7 in this example)

 That's why if you hover over the actual number 5 in that example it will
 light up, but if you're in the background area overlap, the number 7 will
 light up.

 http://www.w3.org/TR/CSS21/zindex.html

 Therefore you do have to know about all the cells at the position, since
 when you're in the foreground phase of hit testing, cell 7 will say Nope,
 you're not inside my foreground, and so you then need to check cell 5,
 which will say Yes, you are inside my foreground.

 The grid structure just isn't expressive enough.  It needs to be patched to
 track all cells in a given row/column instead of just the first one.

 dave
 (hy...@apple.com)


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Table rendering and colspans

[Fady Samuel]

Hi,

I have a quick question (*crosses fingers*) about colspans in webkit. I've
attached a simple example, t8_simple. In this example, we have two rows. One
with one cell with a colspan of 5, and the other has two cells, the first
having a colspan of 1, and the second having a colspan of 3. Thus, we have
an empty slot in the in grid on the second row. In webkit, this introduces
an extra gap at the end of the second row corresponding to the cell spacing
between the second cell and the empty cell. This gap does not appear on
Firefox or Opera.

I've found yet another inconsistency (see attached t9_simple). As you
increase colspan on the first cell, in webkit the gap in the second row
grows by a pixel or two. Firefox remains unchanged. In Opera, the whole
table width increases. What is the correct behavior?

I'm currently reworking chunks of the table rendering code for performance
reasons so it would be nice to know if these are legitimate table rendering
bugs that should be ironed out or the current webkit behavior is good.

Thanks,

Fady Samuel
Title: Blah


  


  

  cell 1,1

  
  

  cell 2,1


  cell 2,2

  

  

Title: Blah


  


  

  cell 1,1

  
  

  cell 2,1


  cell 2,2

  

  

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table rendering and colspans

[Fady Samuel]

Sorry, I was playing around with t8_simple at the last minute and I changed
the colspan. I have attached the test as I described it.

Fady

On Thu, May 27, 2010 at 3:16 PM, Fady Samuel fsam...@google.com wrote:

 Hi,

 I have a quick question (*crosses fingers*) about colspans in webkit. I've
 attached a simple example, t8_simple. In this example, we have two rows. One
 with one cell with a colspan of 5, and the other has two cells, the first
 having a colspan of 1, and the second having a colspan of 3. Thus, we have
 an empty slot in the in grid on the second row. In webkit, this introduces
 an extra gap at the end of the second row corresponding to the cell spacing
 between the second cell and the empty cell. This gap does not appear on
 Firefox or Opera.

 I've found yet another inconsistency (see attached t9_simple). As you
 increase colspan on the first cell, in webkit the gap in the second row
 grows by a pixel or two. Firefox remains unchanged. In Opera, the whole
 table width increases. What is the correct behavior?

 I'm currently reworking chunks of the table rendering code for performance
 reasons so it would be nice to know if these are legitimate table rendering
 bugs that should be ironed out or the current webkit behavior is good.

 Thanks,

 Fady Samuel

Title: Blah


  


  

  cell 1,1

  
  

  cell 2,1


  cell 2,2

  

  

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table rendering and colspans

[Fady Samuel]

I just tested this with IE8. It appears IE's behavior matches Opera's in
these cases.

Fady

On Thu, May 27, 2010 at 3:25 PM, Fady Samuel fsam...@google.com wrote:

 Sorry, I was playing around with t8_simple at the last minute and I changed
 the colspan. I have attached the test as I described it.

 Fady


 On Thu, May 27, 2010 at 3:16 PM, Fady Samuel fsam...@google.com wrote:

 Hi,

 I have a quick question (*crosses fingers*) about colspans in webkit. I've
 attached a simple example, t8_simple. In this example, we have two rows. One
 with one cell with a colspan of 5, and the other has two cells, the first
 having a colspan of 1, and the second having a colspan of 3. Thus, we have
 an empty slot in the in grid on the second row. In webkit, this introduces
 an extra gap at the end of the second row corresponding to the cell spacing
 between the second cell and the empty cell. This gap does not appear on
 Firefox or Opera.

 I've found yet another inconsistency (see attached t9_simple). As you
 increase colspan on the first cell, in webkit the gap in the second row
 grows by a pixel or two. Firefox remains unchanged. In Opera, the whole
 table width increases. What is the correct behavior?

 I'm currently reworking chunks of the table rendering code for performance
 reasons so it would be nice to know if these are legitimate table rendering
 bugs that should be ironed out or the current webkit behavior is good.

 Thanks,

 Fady Samuel



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

So I have an implementation that seems to be working (still testing edge
cases). I found that m_grid, in RenderTableSection, is a VectorRowStruct
and each RowStruct contains a row which knows about the columns of that row.

Thus this is effectively a dense 2D table of Cells. I use this for hit
testing. By the time nodeAtPoint has been called we've already computed the
dimensions and location of the rows and cells and all RenderTableCells are
reachable through m_grid. Thus, I do a binary search on the rows, comparing
their y position with the mouse's y pos to find which row to consider.

Then once a row has been picked, I do a binsearch on the columns within that
row, looking at the x positions, and then I pass the event to the nearest
cell. Of course, the actual mouse position may fall just outside of a cell
(it may lie over spacing between cells for example) but in that case the
RenderTableCell node would simply ignore the event.

I'm still doing testing (regression + perf). For cells with rowspan  1, it
seems that the a pointer to the Cell node is placed in each row its
associated with in m_grid. For cells with colspan  1, it seems that only
the first column that the cell occupies is not NULL...so I just linear scan
back to the first non NULL position if I encounter that. I suppose I could
optimize that further by filling all of m_grid appropriately and changing
code elsewhere to accommodate.

CSS transformations on tables seem to all automagically work, it seems all
the coordinate system transformation code all happens before we reach
RenderTableSection::nodeAtPoint.

Does it sound like I'm missing anything important? I've just started playing
with the webkit source a couple of weeks ago, so I may have missed something
silly.

Thanks,

Fady

On Tue, May 18, 2010 at 3:03 PM, Fady Samuel fsam...@google.com wrote:

 Ohh I see, I was confused about this line in RenderTable:

 1138 if (!hasOverflowClip() || overflowClipRect(tx, ty).contains(xPos,
 yPos)) {

 It seems that the default case is to visit all the children as
 hasOverflowClip() is false.

 Thanks for the explanation David.

 I will look into optimizing hit testing to avoid visiting all children.

 Thanks again,

 Fady


 On Tue, May 18, 2010 at 2:57 PM, David Hyatt hy...@apple.com wrote:


 On May 18, 2010, at 12:52 PM, Fady Samuel wrote:
  Hi all,
 
  I'm looking at table hit testing, and in all the simple test cases I've
 tried, it seems to show that RenderTable never has the flag
 m_hasOverflowClip set to true. What this means is that all the table's
 children are ALWAYS checked on all mouse events. For large tables, or pages
 with many tables, this sounds awfully taxing on the processor for no good
 reason.
 
  See RenderTable::nodeAtPoint in
 third_party/WebKit/WebCore/rendering/RenderTable.cpp . It seems
 hasOverflowClip() is always false.
 
  Can we not compute a bounding box for the table on layout? Are there any
 complications here that I should be aware of that resulted in this
 inefficient solution?

 m_hasOverflowClip is about overflow:auto/scroll/hidden.  The default value
 for overflow is visible, so that's not going to be set and isn't really
 relevant to your problem.

 Both RenderTable and RenderTableSection need to have their nodeAtPoint
 methods patched to be like their paint methods instead.  Both of those
 methods would get much more efficient if we did that.

 dave
 (hy...@apple.com)



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

CSS is Awesome indeed :P

So are you against fast paths? It seems if we don't have overflow, we can do
binary searches for hit testing, right? Unless I'm yet again missing
something fundamental? Sorry about that. :P

Fady

On Thu, May 20, 2010 at 2:56 PM, David Hyatt hy...@apple.com wrote:

 You can't binary search if there are overflowing cells in any section.  You
 have to go in order (from last to first) in that case, since you may hit
 test something.

 In general you won't find binary searches, etc. in most hit testing code
 because of overflow concerns.

 http://www.zazzle.com/css_is_awesome_mug-168716435071981928

 I strongly recommend not writing code that deviates from what the paint
 methods do.  In general the nodeAtPoint version of a function should be just
 like the paint version of a function, just from last to first rather than
 first to last.

 Any time hit testing and painting deviate, you have a problem, since the
 painting stack order and the hit testing stack order obviously need to
 agree.

 Basically I'd expect a first patch to synchronize the two methods (paint
 and nodeAtPoint), and then if you want to optimize further, I'd expect to
 see optimizations to both painting and hit testing made together.

 dave
 (hy...@apple.com)

 On May 20, 2010, at 10:58 AM, Fady Samuel wrote:

 So I have an implementation that seems to be working (still testing edge
 cases). I found that m_grid, in RenderTableSection, is a VectorRowStruct
 and each RowStruct contains a row which knows about the columns of that row.

 Thus this is effectively a dense 2D table of Cells. I use this for hit
 testing. By the time nodeAtPoint has been called we've already computed the
 dimensions and location of the rows and cells and all RenderTableCells are
 reachable through m_grid. Thus, I do a binary search on the rows, comparing
 their y position with the mouse's y pos to find which row to consider.

 Then once a row has been picked, I do a binsearch on the columns within
 that row, looking at the x positions, and then I pass the event to the
 nearest cell. Of course, the actual mouse position may fall just outside of
 a cell (it may lie over spacing between cells for example) but in that case
 the RenderTableCell node would simply ignore the event.

 I'm still doing testing (regression + perf). For cells with rowspan  1, it
 seems that the a pointer to the Cell node is placed in each row its
 associated with in m_grid. For cells with colspan  1, it seems that only
 the first column that the cell occupies is not NULL...so I just linear scan
 back to the first non NULL position if I encounter that. I suppose I could
 optimize that further by filling all of m_grid appropriately and changing
 code elsewhere to accommodate.

 CSS transformations on tables seem to all automagically work, it seems all
 the coordinate system transformation code all happens before we reach
 RenderTableSection::nodeAtPoint.

 Does it sound like I'm missing anything important? I've just started
 playing with the webkit source a couple of weeks ago, so I may have missed
 something silly.

 Thanks,

 Fady

 On Tue, May 18, 2010 at 3:03 PM, Fady Samuel fsam...@google.com wrote:

 Ohh I see, I was confused about this line in RenderTable:

 1138 if (!hasOverflowClip() || overflowClipRect(tx, ty).contains(xPos,
 yPos)) {

 It seems that the default case is to visit all the children as
 hasOverflowClip() is false.

 Thanks for the explanation David.

 I will look into optimizing hit testing to avoid visiting all children.

 Thanks again,

 Fady


 On Tue, May 18, 2010 at 2:57 PM, David Hyatt hy...@apple.com wrote:


 On May 18, 2010, at 12:52 PM, Fady Samuel wrote:
  Hi all,
 
  I'm looking at table hit testing, and in all the simple test cases I've
 tried, it seems to show that RenderTable never has the flag
 m_hasOverflowClip set to true. What this means is that all the table's
 children are ALWAYS checked on all mouse events. For large tables, or pages
 with many tables, this sounds awfully taxing on the processor for no good
 reason.
 
  See RenderTable::nodeAtPoint in
 third_party/WebKit/WebCore/rendering/RenderTable.cpp . It seems
 hasOverflowClip() is always false.
 
  Can we not compute a bounding box for the table on layout? Are there
 any complications here that I should be aware of that resulted in this
 inefficient solution?

 m_hasOverflowClip is about overflow:auto/scroll/hidden.  The default
 value for overflow is visible, so that's not going to be set and isn't
 really relevant to your problem.

 Both RenderTable and RenderTableSection need to have their nodeAtPoint
 methods patched to be like their paint methods instead.  Both of those
 methods would get much more efficient if we did that.

 dave
 (hy...@apple.com)





___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

So what are the rules for stacking here? do the cells stack in the order in
which they appear in the HTML?

Cell 7 is defined after cell 5 and therefore it owns that position?

Thanks,

Fady

On Thu, May 20, 2010 at 4:18 PM, David Hyatt hy...@apple.com wrote:

 On May 20, 2010, at 3:07 PM, David Hyatt wrote:

 If we could properly detect those degenerate cases, then you could probably
 get away with binary search, but until we do that, I don't think you can.


 Here's an example:

 STYLE TD:hover { color: green } /STYLE  TABLE border=1 TRTD1
 TD2 TD3 TRTD4 TD rowspan=25 TD6 TRTD colspan=27 TD9
 /TABLE

 In this example, cell 5 and cell 7 actually overlap and share a position in
 the grid.  You can see that hit testing works properly and gives cell 7
 precedence over cell 5 (e.g., if you're in overlapping background areas of
 the cell, then cell 7 wins).


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

I'm still confused about the proper ordering of painting/hit testing cells
for a given grid position.

In the example you provided, David, WHY does cell 7 have precedence over
cell 5? Is it based on the order they're defined?

Thanks,

Fady

On Thu, May 20, 2010 at 4:47 PM, Peter Kasting pkast...@google.com wrote:

 On Thu, May 20, 2010 at 1:38 PM, David Hyatt hy...@apple.com wrote:

 This is all coming back to me now.

 When two cells occupy the same position in the grid, only one of them gets
 stored at that position.  The current code puts the first cell encountered
 into the grid. This means if you ask what cell is present in row 3 and
 column 2 of the above example, you'll get back cell #5.

 This actually makes the grid unsuitable for hit testing and is why I
 hadn't converted nodeAtPoint over to use the grid yet.

 The grid needs to be fixed so that it records all of the cells present at
 a given position.  If it did that, then you could binary search as long as
 there are no overflowing cells, since you'd then be able to see all the
 cells that actually occupy a given grid position.

 The paint method uses the grid and actually has a repaint bug, since if
 you make the repaint rect tight enough, it won't think it has to paint the
 second and subsequent cells that occupy a given grid position.


 It sounds like your recommended steps are:
 * First fix the grid to record all the cells at a given position.  This
 fixes a repaint bug.
 * Then change the hit-testing to work just like painting.  This makes hit
 testing more efficient and ensures hit testing and painting agree.
 * Then optimize hit-testing to binary-search if there are no overflowing
 cells.  (And optimize painting similarly?  I dunno what that would mean,
 though--just going off the optimize together comment earlier.)

 Is this a good summary?

 PK

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Table hit testing

[Fady Samuel]

Hi all,

I'm looking at table hit testing, and in all the simple test cases I've
tried, it seems to show that RenderTable never has the flag
m_hasOverflowClip set to true. What this means is that all the table's
children are ALWAYS checked on all mouse events. For large tables, or pages
with many tables, this sounds awfully taxing on the processor for no good
reason.

See RenderTable::nodeAtPoint in
third_party/WebKit/WebCore/rendering/RenderTable.cpp . It seems
hasOverflowClip() is always false.

Can we not compute a bounding box for the table on layout? Are there any
complications here that I should be aware of that resulted in this
inefficient solution?

Thanks,

Fady Samuel
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Table hit testing

[Fady Samuel]

Ohh I see, I was confused about this line in RenderTable:

1138 if (!hasOverflowClip() || overflowClipRect(tx, ty).contains(xPos,
yPos)) {

It seems that the default case is to visit all the children as
hasOverflowClip() is false.

Thanks for the explanation David.

I will look into optimizing hit testing to avoid visiting all children.

Thanks again,

Fady

On Tue, May 18, 2010 at 2:57 PM, David Hyatt hy...@apple.com wrote:


 On May 18, 2010, at 12:52 PM, Fady Samuel wrote:
  Hi all,
 
  I'm looking at table hit testing, and in all the simple test cases I've
 tried, it seems to show that RenderTable never has the flag
 m_hasOverflowClip set to true. What this means is that all the table's
 children are ALWAYS checked on all mouse events. For large tables, or pages
 with many tables, this sounds awfully taxing on the processor for no good
 reason.
 
  See RenderTable::nodeAtPoint in
 third_party/WebKit/WebCore/rendering/RenderTable.cpp . It seems
 hasOverflowClip() is always false.
 
  Can we not compute a bounding box for the table on layout? Are there any
 complications here that I should be aware of that resulted in this
 inefficient solution?

 m_hasOverflowClip is about overflow:auto/scroll/hidden.  The default value
 for overflow is visible, so that's not going to be set and isn't really
 relevant to your problem.

 Both RenderTable and RenderTableSection need to have their nodeAtPoint
 methods patched to be like their paint methods instead.  Both of those
 methods would get much more efficient if we did that.

 dave
 (hy...@apple.com)


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev