370492 - Stop using views for event.pageX/Y, event.layerX/Y, image.x/y

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Eli Friedman]

Attachment: Patch

Per summary, we should stop using views because they are not reliable and we are evetually going to get rid of them.

The pageX/Y changes are straightforward; it's just switching from using nsIScrollableView to nsIScrollableFrame.

The other changes are trickier.  The way I'm currently doing it, it could break compatability if people are using layerX/Y or image.x/y in conjuction with the overflow properties.  Not sure how we want to deal with this.

Comment 1

[Eli Friedman]

Attachment: Patch v2

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  nsIFrame* layer;
+  nsIFrame* layerParent = frame;
+  do {
+    layer = layerParent;
+    if (layer->GetStyleDisplay()->IsAbsolutelyPositioned())
+      break;
+    layerParent = layer->GetParent();
+  } while (layerParent &&
+           (layerParent->GetType() != nsGkAtoms::scrollFrame));

I'd prefer this a bit simpler, say

  nsIFrame* layer;
  for (layer = frame; layer; layer = layer->GetParent()) {
    if (layer->GetStyleDisplay()->IsAbsolutelyPositioned() ||
        (layer->GetParent() && layer->GetParent()->GetType() == nsGkAtoms::scrollFrame))
      break;
  }

Actually, I think nsLayoutUtils should have a method IsLayer(nsIFrame*) which encapsulates our definition of a "layer", and we should use it from GetLayerPoint too. It should probably be IsPositioned || parent->GetType() == nsGkAtoms::scrollFrame.

+    nsIScrollableFrame* scrollframe;
+    nsPoint pt(0,0);
+    CallQueryInterface(mPresContext->PresShell()->GetRootScrollFrame(),
+                       &scrollframe);

Let's add a method GetRootScrollFrameAsScrollable to nsIPresShell. Some of the existing callers to GetRootScrollFrame can be retargeted to use that method.

Comment 3

[Eli Friedman]

Attachment: Patch v3

Okay; added a function to nsLayoutUtils to do the loop, and added the function to nsIPresShell.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 255556 [details] [diff] [review]
Patch v3

+  nsresult rv;
+  nsIFrame* frame = GetRootScrollFrame();
+  nsIScrollableFrame* scrollableFrame;
+  rv = CallQueryInterface(frame, &scrollableFrame);
+  NS_ENSURE_SUCCESS(rv, nsnull);
+  return scrollableFrame;

You should check for null before calling CallQueryInterface. And I wouldn't bother checking rv, we can just return scrollableFrame.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

And at some point we should audit the callers to GetRootScrollFrame and convert some of them to GetRootScrollFrameAsScrollable.

Comment 6

[Eli Friedman]

Checked in.