Bug 619955. 'pointer-events' broken on SVG text. r=longsonr, a=roc.

--HG--
extra : rebase_source : 69ebfac4019f926014c957c9984cc6d6795016b3

content/svg/content/test/test_pointer-events.xhtml

@@ -15,6 +15,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=619959
 SimpleTest.waitForExplicitFinish();
 
 var pointer_events_values = [
+  'auto',
   'visiblePainted',
   'visibleFill',
   'visibleStroke',
@@ -46,14 +47,15 @@ var visibility_values = [
 
 /**
  * List of attributes and various values for which we want to test permutations
- * when hit testing stroke and fill.
+ * when hit testing a pointer event that is over an element's fill area,
+ * stroke area, or both (where they overlap).
  *
  * We're using an array of objects so that we have control over the order in
  * which permutations are tested.
  *
  * TODO: test the effect of clipping, masking, filters, markers, etc.
  */
-var hit_test_attr_values = {
+var hit_test_inputs = {
   fill: [
     { name: 'pointer-events',  values: pointer_events_values },
     { name: 'fill',            values: paint_values },
@@ -67,6 +69,15 @@ var hit_test_attr_values = {
     { name: 'stroke-opacity',  values: opacity_values },
     { name: 'opacity',         values: opacity_values },
     { name: 'visibility',      values: visibility_values }
+  ],
+  both: [
+    { name: 'pointer-events',  values: pointer_events_values },
+    { name: 'fill',            values: paint_values },
+    { name: 'fill-opacity',    values: opacity_values },
+    { name: 'stroke',          values: paint_values },
+    { name: 'stroke-opacity',  values: opacity_values },
+    { name: 'opacity',         values: opacity_values },
+    { name: 'visibility',      values: visibility_values }
   ]
 }
 
@@ -88,6 +99,16 @@ var hit_test_attr_values = {
  * of the values listed in the given array.
  */
 var hit_conditions = {
+  auto: {
+    'fill-intercepts-iff': {
+      'visibility': ['visible'],
+      'fill!': ['none']
+    },
+    'stroke-intercepts-iff': {
+      'visibility': ['visible'],
+      'stroke!': ['none']
+    }
+  },
   visiblePainted: {
     'fill-intercepts-iff': {
       'visibility': ['visible'],
@@ -151,36 +172,46 @@ var hit_conditions = {
   }
 }
 
+// bit flags
+var POINT_OVER_FILL   = 0x1;
+var POINT_OVER_STROKE = 0x2;
+
 /**
- * Examine the current attribute values and return true if the specified target
- * (fill or stroke) is expected to intercept events, or else return false.
+ * Examine the element's attribute values and, based on the area(s) of the
+ * element that the pointer event is over (fill and/or stroke areas), return
+ * true if the element is expected to intercept the event, otherwise false.
  */
-function hit_expected(target /* {stroke|fill} */, attributes)
+function hit_expected(element, over /* bit flags indicating which area(s) of the element the pointer is over */)
 {
-  var intercepts_iff =
-    hit_conditions[attributes['pointer-events']][target + '-intercepts-iff'];
+  function expect_hit(target){
+    var intercepts_iff =
+      hit_conditions[element.getAttribute('pointer-events')][target + '-intercepts-iff'];
 
-  if (!intercepts_iff) {
-    return false; // never intercepts events
+    if (!intercepts_iff) {
+      return false; // never intercepts events
+    }
+
+    for (var attr in intercepts_iff) {
+      var vals = intercepts_iff[attr];  // must get this before we adjust 'attr'
+      var invert = false;
+      if (attr.substr(-1) == '!') {
+        invert = true;
+        attr = attr.substr(0, attr.length-1);
+      }
+      var match = vals.indexOf(element.getAttribute(attr)) > -1;
+      if (invert) {
+        match = !match;
+      }
+      if (!match) {
+        return false;
+      }
+    }
+
+    return true;
   }
 
-  for (var attr in intercepts_iff) {
-    var vals = intercepts_iff[attr];  // must get this before we adjust 'attr'
-    var invert = false;
-    if (attr.substr(-1) == '!') {
-      invert = true;
-      attr = attr.substr(0, attr.length-1);
-    }
-    var match = vals.indexOf(attributes[attr]) > -1;
-    if (invert) {
-      match = !match;
-    }
-    if (!match) {
-      return false;
-    }
-  }
-
-  return true;
+  return (over & POINT_OVER_FILL) != 0   && expect_hit('fill') ||
+         (over & POINT_OVER_STROKE) != 0 && expect_hit('stroke');
 }
 
 function for_all_permutations(inputs, callback)
@@ -201,55 +232,95 @@ function for_all_permutations(inputs, callback)
   callback(current_permutation);
 }
 
-function log_msg(target, id, attributes)
+function make_log_msg(over, tag, attributes)
 {
-  var msg = 'Check if events are intercepted by '+target+' on '+id+' for';
+  var target;
+  if (over == (POINT_OVER_FILL | POINT_OVER_STROKE)) {
+    target = 'fill and stroke';
+  } else if (over == POINT_OVER_FILL) {
+    target = 'fill';
+  } else if (over == POINT_OVER_STROKE) {
+    target = 'stroke';
+  } else {
+    throw "unexpected bit combination in 'over'";
+  }
+  var msg = 'Check if events are intercepted at a point over the '+target+' on <'+tag+'> for';
   for (var attr in attributes) {
     msg += ' '+attr+'='+attributes[attr];
   }
   return msg;
 }
 
-function run_tests()
-{
-  var div = document.getElementById("div");
-  var dx = div.offsetLeft;
-  var dy = div.offsetTop;
+var dx, dy; // offset of <svg> element from pointer coordinates origin
 
-  var id, element, target;  // target is 'fill' or 'stroke'
-  var x, y;  // coordinates to hit test
+function test_element(id, x, y, over /* bit flags indicating which area(s) of the element the pointer is over */)
+{
+  var element = document.getElementById(id);
+  var tag = element.tagName;
 
   function test_permutation(attributes) {
     for (var attr in attributes) {
       element.setAttribute(attr, attributes[attr]);
     }
-    var hit = document.elementFromPoint(dx + x, dy + y) == element;
-    is(hit, hit_expected(target, attributes), log_msg(target, id, attributes));
+    var hits = document.elementFromPoint(dx + x, dy + y) == element;
+    var msg = make_log_msg(over, tag, attributes);
+
+    is(hits, hit_expected(element, over), msg);
   }
 
-  // To reduce the chance of bogus results
-  function clear_attributes_for_next_test(inputs) {
-    for (var i = 0; i < inputs.length; ++i) {
-      element.removeAttribute(inputs[i].name);
-    }
-    element.setAttribute('fill', 'none');
-    element.setAttribute('stroke', 'none');
+  var inputs;
+  if (over == (POINT_OVER_FILL | POINT_OVER_STROKE)) {
+    inputs = hit_test_inputs['both'];
+  } else if (over == POINT_OVER_FILL) {
+    inputs = hit_test_inputs['fill'];
+  } else if (over == POINT_OVER_STROKE) {
+    inputs = hit_test_inputs['stroke'];
+  } else {
+    throw "unexpected bit combination in 'over'";
   }
 
-  id = 'rect';
-  element = document.getElementById(id);
+  for_all_permutations(inputs, test_permutation);
 
-  var target = 'fill';
-  x = 30;
-  y = 30;
-  for_all_permutations(hit_test_attr_values[target], test_permutation);
-  clear_attributes_for_next_test(hit_test_attr_values[target]);
+  // To reduce the chance of bogus results in subsequent tests:
+  element.setAttribute('fill', 'none');
+  element.setAttribute('stroke', 'none');
+}
 
-  var target = 'stroke';
-  x = 5;
-  y = 5;
-  for_all_permutations(hit_test_attr_values[target], test_permutation);
-  clear_attributes_for_next_test(hit_test_attr_values[target]);
+function run_tests()
+{
+  var div = document.getElementById("div");
+  dx = div.offsetLeft;
+  dy = div.offsetTop;
+
+  test_element('rect', 30, 30, POINT_OVER_FILL);
+  test_element('rect', 5, 5, POINT_OVER_STROKE);
+
+  // The SVG 1.1 spec essentially says that, for text, hit testing is done
+  // against the character cells of the text, and not the fill and stroke as
+  // you might expect for a normal graphics element like <path>. See the
+  // paragraph starting "For text elements..." in this section:
+  //
+  //   http://www.w3.org/TR/SVG11/interact.html#PointerEventsProperty
+  //
+  // This requirement essentially means that for the purposes of hit testing
+  // the fill and stroke areas are the same area - the character cell. (At
+  // least until we support having any fill or stroke that lies outside the
+  // character cells intercept events like Opera does - see below.) Thus, for
+  // text, when a pointer event is over a character cell it is essentially over
+  // both the fill and stroke at the same time. That's the reason we pass both
+  // the POINT_OVER_FILL and POINT_OVER_STROKE bits in test_element's 'over'
+  // argument below. It's also the reason why we only test one point in the
+  // text rather than having separate tests for fill and stroke.
+  //
+  // For hit testing of text, Opera essentially treats fill and stroke like it
+  // would on any normal element, but it adds the character cells of glyhs to
+  // both the glyphs' fill AND stroke. I think this is what we should do too.
+  // It's compatible with the letter of the SVG 1.1 rules, and it allows any
+  // parts of a glyph that are outside the glyph's character cells to also
+  // intercept events in the normal way. When we make that change we'll be able
+  // to add separate fill and stroke tests for text below.
+
+  test_element('text', 210, 30, POINT_OVER_FILL | POINT_OVER_STROKE);
 
   SimpleTest.finish();
 }
@@ -264,6 +335,7 @@ function run_tests()
 
   <svg xmlns="http://www.w3.org/2000/svg" id="svg">
     <rect id="rect" x="10" y="10" width="40" height="40" stroke-width="20"/>
+    <text id="text" x="190" y="50" font-size="40px" stroke-width="20">X</text>
   </svg>
 
 </div>

layout/svg/base/src/nsSVGGeometryFrame.cpp

@@ -325,3 +325,62 @@ nsSVGGeometryFrame::SetupCairoStroke(gfxContext *aContext)
 
   return PR_TRUE;
 }
+
+PRUint16
+nsSVGGeometryFrame::GetHittestMask()
+{
+  PRUint16 mask = 0;
+
+  switch(GetStyleVisibility()->mPointerEvents) {
+  case NS_STYLE_POINTER_EVENTS_NONE:
+    break;
+  case NS_STYLE_POINTER_EVENTS_AUTO:
+  case NS_STYLE_POINTER_EVENTS_VISIBLEPAINTED:
+    if (GetStyleVisibility()->IsVisible()) {
+      if (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None)
+        mask |= HITTEST_MASK_FILL;
+      if (GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None)
+        mask |= HITTEST_MASK_STROKE;
+      if (GetStyleSVG()->mStrokeOpacity > 0)
+        mask |= HITTEST_MASK_CHECK_MRECT;
+    }
+    break;
+  case NS_STYLE_POINTER_EVENTS_VISIBLEFILL:
+    if (GetStyleVisibility()->IsVisible()) {
+      mask |= HITTEST_MASK_FILL;
+    }
+    break;
+  case NS_STYLE_POINTER_EVENTS_VISIBLESTROKE:
+    if (GetStyleVisibility()->IsVisible()) {
+      mask |= HITTEST_MASK_STROKE;
+    }
+    break;
+  case NS_STYLE_POINTER_EVENTS_VISIBLE:
+    if (GetStyleVisibility()->IsVisible()) {
+      mask |= HITTEST_MASK_FILL | HITTEST_MASK_STROKE;
+    }
+    break;
+  case NS_STYLE_POINTER_EVENTS_PAINTED:
+    if (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None)
+      mask |= HITTEST_MASK_FILL;
+    if (GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None)
+      mask |= HITTEST_MASK_STROKE;
+    if (GetStyleSVG()->mStrokeOpacity)
+      mask |= HITTEST_MASK_CHECK_MRECT;
+    break;
+  case NS_STYLE_POINTER_EVENTS_FILL:
+    mask |= HITTEST_MASK_FILL;
+    break;
+  case NS_STYLE_POINTER_EVENTS_STROKE:
+    mask |= HITTEST_MASK_STROKE;
+    break;
+  case NS_STYLE_POINTER_EVENTS_ALL:
+    mask |= HITTEST_MASK_FILL | HITTEST_MASK_STROKE;
+    break;
+  default:
+    NS_ERROR("not reached");
+    break;
+  }
+
+  return mask;
+}

layout/svg/base/src/nsSVGGeometryFrame.h

@@ -46,6 +46,10 @@ class gfxContext;
 
 typedef nsFrame nsSVGGeometryFrameBase;
 
+#define HITTEST_MASK_FILL        0x01
+#define HITTEST_MASK_STROKE      0x02
+#define HITTEST_MASK_CHECK_MRECT 0x04
+
 /* nsSVGGeometryFrame is a base class for SVG objects that directly
  * have geometry (circle, ellipse, line, polyline, polygon, path, and
  * glyph frames).  It knows how to convert the style information into
@@ -102,6 +106,7 @@ public:
 protected:
   nsSVGPaintServerFrame *GetPaintServer(const nsStyleSVGPaint *aPaint,
                                         const FramePropertyDescriptor *aProperty);
+  virtual PRUint16 GetHittestMask();
 
 private:
   nsresult GetStrokeDashArray(double **arr, PRUint32 *count);

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -62,7 +62,11 @@ struct CharacterPosition {
   gfxFloat angle;
   PRBool draw;
 };
-  
+
+static gfxContext* MakeTmpCtx() {
+  return new gfxContext(gfxPlatform::GetPlatform()->ScreenReferenceSurface());
+}
+
 /**
  * This is a do-it-all helper class. It supports iterating through the
  * drawable character clusters of a string. For each cluster, it can set up
@@ -412,42 +416,50 @@ nsSVGGlyphFrame::PaintSVG(nsSVGRenderState *aContext,
 NS_IMETHODIMP_(nsIFrame*)
 nsSVGGlyphFrame::GetFrameForPoint(const nsPoint &aPoint)
 {
-  if (!mRect.Contains(aPoint))
+  PRUint16 mask = GetHittestMask();
+  if (!mask) {
     return nsnull;
-
-  PRBool events = PR_FALSE;
-  switch (GetStyleVisibility()->mPointerEvents) {
-    case NS_STYLE_POINTER_EVENTS_NONE:
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLEPAINTED:
-    case NS_STYLE_POINTER_EVENTS_AUTO:
-      if (GetStyleVisibility()->IsVisible() &&
-          (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None ||
-           GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None))
-        events = PR_TRUE;
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLEFILL:
-    case NS_STYLE_POINTER_EVENTS_VISIBLESTROKE:
-    case NS_STYLE_POINTER_EVENTS_VISIBLE:
-      if (GetStyleVisibility()->IsVisible())
-        events = PR_TRUE;
-      break;
-    case NS_STYLE_POINTER_EVENTS_PAINTED:
-      if (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None ||
-          GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None)
-        events = PR_TRUE;
-      break;
-    case NS_STYLE_POINTER_EVENTS_FILL:
-    case NS_STYLE_POINTER_EVENTS_STROKE:
-    case NS_STYLE_POINTER_EVENTS_ALL:
-      events = PR_TRUE;
-      break;
-    default:
-      NS_ERROR("not reached");
-      break;
   }
 
-  if (events && ContainsPoint(aPoint))
+  nsRefPtr<gfxContext> context = MakeTmpCtx();
+  SetupGlobalTransform(context);
+  CharacterIterator iter(this, PR_TRUE);
+  iter.SetInitialMatrix(context);
+
+  // The SVG 1.1 spec says that text is hit tested against the character cells
+  // of the text, not the fill and stroke. See the section starting "For text
+  // elements..." here:
+  //
+  //   http://www.w3.org/TR/SVG11/interact.html#PointerEventsProperty
+  //
+  // Currently we just test the character cells if GetHittestMask says we're
+  // supposed to be testing either the fill OR the stroke:
+
+  PRInt32 i;
+  while ((i = iter.NextCluster()) >= 0) {
+    gfxTextRun::Metrics metrics =
+    mTextRun->MeasureText(i, iter.ClusterLength(),
+                          gfxFont::LOOSE_INK_EXTENTS, nsnull, nsnull);
+    iter.SetupForMetrics(context);
+    context->Rectangle(metrics.mBoundingBox);
+  }
+
+  gfxPoint userSpacePoint =
+    context->DeviceToUser(gfxPoint(PresContext()->AppUnitsToGfxUnits(aPoint.x),
+                                   PresContext()->AppUnitsToGfxUnits(aPoint.y)));
+
+  PRBool isHit = PR_FALSE;
+  if (mask & HITTEST_MASK_FILL || mask & HITTEST_MASK_STROKE) {
+    isHit = context->PointInFill(userSpacePoint);
+  }
+
+  // If isHit is false, we may also want to fill and stroke the text to check
+  // whether the pointer is over an area of fill or stroke that lies outside
+  // the character cells. (With a thick stroke, or with fonts like Zapfino, such
+  // areas may be very significant.) This is what Opera appears to do, but
+  // currently we do not.
+
+  if (isHit && nsSVGUtils::HitTestClip(this, aPoint))
     return this;
 
   return nsnull;
@@ -459,11 +471,6 @@ nsSVGGlyphFrame::GetCoveredRegion()
   return mRect;
 }
 
-static gfxContext *
-MakeTmpCtx() {
-  return new gfxContext(gfxPlatform::GetPlatform()->ScreenReferenceSurface());
-}
-
 NS_IMETHODIMP
 nsSVGGlyphFrame::UpdateCoveredRegion()
 {
@@ -1472,28 +1479,6 @@ nsSVGGlyphFrame::NotifyGlyphMetricsChange()
     containerFrame->NotifyGlyphMetricsChange();
 }
 
-PRBool
-nsSVGGlyphFrame::ContainsPoint(const nsPoint &aPoint)
-{
-  nsRefPtr<gfxContext> tmpCtx = MakeTmpCtx();
-  SetupGlobalTransform(tmpCtx);
-  CharacterIterator iter(this, PR_TRUE);
-  iter.SetInitialMatrix(tmpCtx);
-  
-  PRInt32 i;
-  while ((i = iter.NextCluster()) >= 0) {
-    gfxTextRun::Metrics metrics =
-      mTextRun->MeasureText(i, iter.ClusterLength(),
-                            gfxFont::LOOSE_INK_EXTENTS, nsnull, nsnull);
-    iter.SetupForMetrics(tmpCtx);
-    tmpCtx->Rectangle(metrics.mBoundingBox);
-  }
-
-  tmpCtx->IdentityMatrix();
-  return tmpCtx->PointInFill(gfxPoint(PresContext()->AppUnitsToGfxUnits(aPoint.x),
-                                      PresContext()->AppUnitsToGfxUnits(aPoint.y)));
-}
-
 PRBool
 nsSVGGlyphFrame::GetGlobalTransform(gfxMatrix *aMatrix)
 {

layout/svg/base/src/nsSVGGlyphFrame.h

@@ -208,7 +208,6 @@ protected:
                       gfxContext *aContext);
 
   void NotifyGlyphMetricsChange();
-  PRBool ContainsPoint(const nsPoint &aPoint);
   PRBool GetGlobalTransform(gfxMatrix *aMatrix);
   void SetupGlobalTransform(gfxContext *aContext);
   nsresult GetHighlight(PRUint32 *charnum, PRUint32 *nchars,

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -505,66 +505,3 @@ nsSVGPathGeometryFrame::GeneratePath(gfxContext* aContext,
   aContext->NewPath();
   static_cast<nsSVGPathGeometryElement*>(mContent)->ConstructPath(aContext);
 }
-
-PRUint16
-nsSVGPathGeometryFrame::GetHittestMask()
-{
-  PRUint16 mask = 0;
-
-  switch(GetStyleVisibility()->mPointerEvents) {
-    case NS_STYLE_POINTER_EVENTS_NONE:
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLEPAINTED:
-    case NS_STYLE_POINTER_EVENTS_AUTO:
-      if (GetStyleVisibility()->IsVisible()) {
-        if (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None)
-          mask |= HITTEST_MASK_FILL;
-        if (GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None)
-          mask |= HITTEST_MASK_STROKE;
-        if (GetStyleSVG()->mStrokeOpacity > 0)
-          mask |= HITTEST_MASK_CHECK_MRECT;
-      }
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLEFILL:
-      if (GetStyleVisibility()->IsVisible()) {
-        mask |= HITTEST_MASK_FILL;
-      }
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLESTROKE:
-      if (GetStyleVisibility()->IsVisible()) {
-        mask |= HITTEST_MASK_STROKE;
-      }
-      break;
-    case NS_STYLE_POINTER_EVENTS_VISIBLE:
-      if (GetStyleVisibility()->IsVisible()) {
-        mask |=
-          HITTEST_MASK_FILL |
-          HITTEST_MASK_STROKE;
-      }
-      break;
-    case NS_STYLE_POINTER_EVENTS_PAINTED:
-      if (GetStyleSVG()->mFill.mType != eStyleSVGPaintType_None)
-        mask |= HITTEST_MASK_FILL;
-      if (GetStyleSVG()->mStroke.mType != eStyleSVGPaintType_None)
-        mask |= HITTEST_MASK_STROKE;
-      if (GetStyleSVG()->mStrokeOpacity)
-        mask |= HITTEST_MASK_CHECK_MRECT;
-      break;
-    case NS_STYLE_POINTER_EVENTS_FILL:
-      mask |= HITTEST_MASK_FILL;
-      break;
-    case NS_STYLE_POINTER_EVENTS_STROKE:
-      mask |= HITTEST_MASK_STROKE;
-      break;
-    case NS_STYLE_POINTER_EVENTS_ALL:
-      mask |=
-        HITTEST_MASK_FILL |
-        HITTEST_MASK_STROKE;
-      break;
-    default:
-      NS_ERROR("not reached");
-      break;
-  }
-
-  return mask;
-}

layout/svg/base/src/nsSVGPathGeometryFrame.h

@@ -52,10 +52,6 @@ class nsSVGMarkerProperty;
 
 typedef nsSVGGeometryFrame nsSVGPathGeometryFrameBase;
 
-#define HITTEST_MASK_FILL        0x01
-#define HITTEST_MASK_STROKE      0x02
-#define HITTEST_MASK_CHECK_MRECT 0x04
-
 class nsSVGPathGeometryFrame : public nsSVGPathGeometryFrameBase,
                                public nsISVGChildFrame
 {
@@ -111,7 +107,6 @@ protected:
   NS_IMETHOD_(PRBool) HasValidCoveredRect() { return PR_TRUE; }
 
 protected:
-  virtual PRUint16 GetHittestMask();
   void GeneratePath(gfxContext *aContext,
                     const gfxMatrix *aOverrideTransform = nsnull);