Bug 649142 - Part 4: Convert logical margin properties. r=dbaron

2014-12-31 12:18:14 +11:00 · 2014-12-31 12:18:14 +11:00 · 246dccd863
--- a/browser/devtools/layoutview/view.js
+++ b/browser/devtools/layoutview/view.js
@ -174,17 +174,11 @@ LayoutView.prototype = {
      marginBottom: {selector: ".margin.bottom > span",
                  property: "margin-bottom",
                  value: undefined},
-      // margin-left is a shorthand for some internal properties,
-      // margin-left-ltr-source and margin-left-rtl-source for example. The
-      // real margin value we want is in margin-left-value
      marginLeft: {selector: ".margin.left > span",
                  property: "margin-left",
-                  realProperty: "margin-left-value",
                  value: undefined},
-      // margin-right behaves the same as margin-left
      marginRight: {selector: ".margin.right > span",
                  property: "margin-right",
-                  realProperty: "margin-right-value",
                  value: undefined},
      paddingTop: {selector: ".padding.top > span",
                  property: "padding-top",
@ -261,11 +255,9 @@ LayoutView.prototype = {
   * Called when the user clicks on one of the editable values in the layoutview
   */
  initEditor: function(element, event, dimension) {
-    let { property, realProperty } = dimension;
-    if (!realProperty)
-      realProperty = property;
+    let { property } = dimension;
    let session = new EditingSession(document, this.elementRules);
-    let initialValue = session.getProperty(realProperty);
+    let initialValue = session.getProperty(property);

    let editor = new InplaceEditor({
      element: element,
--- a/dom/html/HTMLBodyElement.cpp
+++ b/dom/html/HTMLBodyElement.cpp
@ -62,10 +62,10 @@ BodyRule::MapRuleInfoInto(nsRuleData* aData)
    if (value && value->Type() == nsAttrValue::eInteger) {
      bodyMarginWidth = value->GetIntegerValue();
      if (bodyMarginWidth < 0) bodyMarginWidth = 0;
-      nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
+      nsCSSValue* marginLeft = aData->ValueForMarginLeft();
      if (marginLeft->GetUnit() == eCSSUnit_Null)
        marginLeft->SetFloatValue((float)bodyMarginWidth, eCSSUnit_Pixel);
-      nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+      nsCSSValue* marginRight = aData->ValueForMarginRight();
      if (marginRight->GetUnit() == eCSSUnit_Null)
        marginRight->SetFloatValue((float)bodyMarginWidth, eCSSUnit_Pixel);
    }
@ -107,7 +107,7 @@ BodyRule::MapRuleInfoInto(nsRuleData* aData)
    if (value && value->Type() == nsAttrValue::eInteger) {
      bodyLeftMargin = value->GetIntegerValue();
      if (bodyLeftMargin < 0) bodyLeftMargin = 0;
-      nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
+      nsCSSValue* marginLeft = aData->ValueForMarginLeft();
      if (marginLeft->GetUnit() == eCSSUnit_Null)
        marginLeft->SetFloatValue((float)bodyLeftMargin, eCSSUnit_Pixel);
    }
@ -117,7 +117,7 @@ BodyRule::MapRuleInfoInto(nsRuleData* aData)
    if (value && value->Type() == nsAttrValue::eInteger) {
      bodyRightMargin = value->GetIntegerValue();
      if (bodyRightMargin < 0) bodyRightMargin = 0;
-      nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+      nsCSSValue* marginRight = aData->ValueForMarginRight();
      if (marginRight->GetUnit() == eCSSUnit_Null)
        marginRight->SetFloatValue((float)bodyRightMargin, eCSSUnit_Pixel);
    }
@ -147,10 +147,10 @@ BodyRule::MapRuleInfoInto(nsRuleData* aData)
      }

      if ((bodyMarginWidth == -1) && (frameMarginWidth >= 0)) {
-        nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
+        nsCSSValue* marginLeft = aData->ValueForMarginLeft();
        if (marginLeft->GetUnit() == eCSSUnit_Null)
          marginLeft->SetFloatValue((float)frameMarginWidth, eCSSUnit_Pixel);
-        nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+        nsCSSValue* marginRight = aData->ValueForMarginRight();
        if (marginRight->GetUnit() == eCSSUnit_Null)
          marginRight->SetFloatValue((float)frameMarginWidth, eCSSUnit_Pixel);
      }
--- a/dom/html/HTMLHRElement.cpp
+++ b/dom/html/HTMLHRElement.cpp
@ -88,8 +88,8 @@ HTMLHRElement::MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
    const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::align);
    if (value && value->Type() == nsAttrValue::eEnum) {
      // Map align attribute into auto side margins
-      nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
-      nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+      nsCSSValue* marginLeft = aData->ValueForMarginLeft();
+      nsCSSValue* marginRight = aData->ValueForMarginRight();
      switch (value->GetEnumValue()) {
      case NS_STYLE_TEXT_ALIGN_LEFT:
        if (marginLeft->GetUnit() == eCSSUnit_Null)
--- a/dom/html/HTMLTableElement.cpp
+++ b/dom/html/HTMLTableElement.cpp
@ -705,10 +705,10 @@ HTMLTableElement::MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
    if (value && value->Type() == nsAttrValue::eEnum) {
      if (value->GetEnumValue() == NS_STYLE_TEXT_ALIGN_CENTER ||
          value->GetEnumValue() == NS_STYLE_TEXT_ALIGN_MOZ_CENTER) {
-        nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
+        nsCSSValue* marginLeft = aData->ValueForMarginLeft();
        if (marginLeft->GetUnit() == eCSSUnit_Null)
          marginLeft->SetAutoValue();
-        nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+        nsCSSValue* marginRight = aData->ValueForMarginRight();
        if (marginRight->GetUnit() == eCSSUnit_Null)
          marginRight->SetAutoValue();
      }
@ -721,10 +721,10 @@ HTMLTableElement::MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
      value = aAttributes->GetAttr(nsGkAtoms::hspace);

      if (value && value->Type() == nsAttrValue::eInteger) {
-        nsCSSValue* marginLeft = aData->ValueForMarginLeftValue();
+        nsCSSValue* marginLeft = aData->ValueForMarginLeft();
        if (marginLeft->GetUnit() == eCSSUnit_Null)
          marginLeft->SetFloatValue((float)value->GetIntegerValue(), eCSSUnit_Pixel); 
-        nsCSSValue* marginRight = aData->ValueForMarginRightValue();
+        nsCSSValue* marginRight = aData->ValueForMarginRight();
        if (marginRight->GetUnit() == eCSSUnit_Null)
          marginRight->SetFloatValue((float)value->GetIntegerValue(), eCSSUnit_Pixel);
      }
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@ -1444,10 +1444,10 @@ nsGenericHTMLElement::MapImageMarginAttributeInto(const nsMappedAttributes* aAtt
      hval.SetPercentValue(value->GetPercentValue());

    if (hval.GetUnit() != eCSSUnit_Null) {
-      nsCSSValue* left = aData->ValueForMarginLeftValue();
+      nsCSSValue* left = aData->ValueForMarginLeft();
      if (left->GetUnit() == eCSSUnit_Null)
        *left = hval;
-      nsCSSValue* right = aData->ValueForMarginRightValue();
+      nsCSSValue* right = aData->ValueForMarginRight();
      if (right->GetUnit() == eCSSUnit_Null)
        *right = hval;
    }
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@ -396,10 +396,6 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue,
      }
      break;
    }
-    case eCSSProperty_margin_left:
-    case eCSSProperty_margin_right:
-    case eCSSProperty_margin_start:
-    case eCSSProperty_margin_end:
    case eCSSProperty_border_left_color:
    case eCSSProperty_border_left_style:
    case eCSSProperty_border_left_width:
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@ -175,6 +175,12 @@ EnsurePhysicalProperty(nsCSSProperty& aProperty, nsRuleData* aRuleData)
  bool ltr = direction == NS_STYLE_DIRECTION_LTR;

  switch (aProperty) {
+    case eCSSProperty_margin_end:
+      aProperty = ltr ? eCSSProperty_margin_right : eCSSProperty_margin_left;
+      break;
+    case eCSSProperty_margin_start:
+      aProperty = ltr ? eCSSProperty_margin_left : eCSSProperty_margin_right;
+      break;
    case eCSSProperty_padding_end:
      aProperty = ltr ? eCSSProperty_padding_right : eCSSProperty_padding_left;
      break;
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@ -9887,18 +9887,6 @@ CSSParserImpl::ParsePropertyByFunction(nsCSSProperty aPropID)
    return ParseListStyle();
  case eCSSProperty_margin:
    return ParseMargin();
-  case eCSSProperty_margin_end:
-    return ParseDirectionalBoxProperty(eCSSProperty_margin_end,
-                                       NS_BOXPROP_SOURCE_LOGICAL);
-  case eCSSProperty_margin_left:
-    return ParseDirectionalBoxProperty(eCSSProperty_margin_left,
-                                       NS_BOXPROP_SOURCE_PHYSICAL);
-  case eCSSProperty_margin_right:
-    return ParseDirectionalBoxProperty(eCSSProperty_margin_right,
-                                       NS_BOXPROP_SOURCE_PHYSICAL);
-  case eCSSProperty_margin_start:
-    return ParseDirectionalBoxProperty(eCSSProperty_margin_start,
-                                       NS_BOXPROP_SOURCE_LOGICAL);
  case eCSSProperty_object_position:
    return ParseObjectPosition();
  case eCSSProperty_outline:
@ -12955,20 +12943,11 @@ CSSParserImpl::ParseMargin()
 {
  static const nsCSSProperty kMarginSideIDs[] = {
    eCSSProperty_margin_top,
-    eCSSProperty_margin_right_value,
+    eCSSProperty_margin_right,
    eCSSProperty_margin_bottom,
-    eCSSProperty_margin_left_value
-  };
-  static const nsCSSProperty kMarginSources[] = {
-    eCSSProperty_margin_left_ltr_source,
-    eCSSProperty_margin_left_rtl_source,
-    eCSSProperty_margin_right_ltr_source,
-    eCSSProperty_margin_right_rtl_source,
-    eCSSProperty_UNKNOWN
+    eCSSProperty_margin_left
  };

-  // do this now, in case 4 values weren't specified
-  InitBoxPropsAsPhysical(kMarginSources);
  return ParseBoxProperties(kMarginSideIDs);
 }

--- a/layout/style/nsCSSPropAliasList.h
+++ b/layout/style/nsCSSPropAliasList.h
@ -147,3 +147,11 @@ CSS_PROP_ALIAS(padding-inline-start,
               padding_start,
               PaddingInlineStart,
               "layout.css.vertical-text.enabled")
+CSS_PROP_ALIAS(margin-inline-end,
+               margin_end,
+               MarginInlineEnd,
+               "layout.css.vertical-text.enabled")
+CSS_PROP_ALIAS(margin-inline-start,
+               margin_start,
+               MarginInlineStart,
+               "layout.css.vertical-text.enabled")
--- a/layout/style/nsCSSPropList.h
+++ b/layout/style/nsCSSPropList.h
@ -2331,152 +2331,66 @@ CSS_PROP_MARGIN(
    nullptr,
    offsetof(nsStyleMargin, mMargin),
    eStyleAnimType_Sides_Bottom)
-CSS_PROP_SHORTHAND(
+CSS_PROP_LOGICAL(
    -moz-margin-end,
    margin_end,
    CSS_PROP_DOMPROP_PREFIXED(MarginEnd),
-    CSS_PROPERTY_PARSE_FUNCTION |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "")
-#ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL
-CSS_PROP_MARGIN(
-    margin-end-value,
-    margin_end_value,
-    MarginEndValue,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
+    CSS_PROPERTY_PARSE_VALUE |
        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
        CSS_PROPERTY_STORES_CALC |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
+        CSS_PROPERTY_APPLIES_TO_PAGE_RULE |
+        CSS_PROPERTY_LOGICAL,
    "",
-    VARIANT_AHLP | VARIANT_CALC, // for internal use
+    VARIANT_AHLP | VARIANT_CALC,
    nullptr,
+    Margin,
    CSS_PROP_NO_OFFSET,
    eStyleAnimType_None)
-#endif
-CSS_PROP_SHORTHAND(
+CSS_PROP_LOGICAL(
+    -moz-margin-start,
+    margin_start,
+    CSS_PROP_DOMPROP_PREFIXED(MarginStart),
+    CSS_PROPERTY_PARSE_VALUE |
+        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
+        CSS_PROPERTY_STORES_CALC |
+        CSS_PROPERTY_APPLIES_TO_PAGE_RULE |
+        CSS_PROPERTY_LOGICAL,
+    "",
+    VARIANT_AHLP | VARIANT_CALC,
+    nullptr,
+    Margin,
+    CSS_PROP_NO_OFFSET,
+    eStyleAnimType_None)
+CSS_PROP_MARGIN(
    margin-left,
    margin_left,
    MarginLeft,
-    CSS_PROPERTY_PARSE_FUNCTION |
+    CSS_PROPERTY_PARSE_VALUE |
        CSS_PROPERTY_UNITLESS_LENGTH_QUIRK |
+        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
+        CSS_PROPERTY_STORES_CALC |
        CSS_PROPERTY_APPLIES_TO_PAGE_RULE |
        CSS_PROPERTY_GETCS_NEEDS_LAYOUT_FLUSH,
-    "")
-#ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL
-CSS_PROP_MARGIN(
-    margin-left-value,
-    margin_left_value,
-    MarginLeftValue,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_REPORT_OTHER_NAME |
-        CSS_PROPERTY_STORES_CALC |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
    "",
-    VARIANT_AHLP | VARIANT_CALC, // for internal use
+    VARIANT_AHLP | VARIANT_CALC,
    nullptr,
    offsetof(nsStyleMargin, mMargin),
    eStyleAnimType_Sides_Left)
 CSS_PROP_MARGIN(
-    margin-left-ltr-source,
-    margin_left_ltr_source,
-    MarginLeftLTRSource,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_DIRECTIONAL_SOURCE |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "",
-    0,
-    kBoxPropSourceKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_None)
-CSS_PROP_MARGIN(
-    margin-left-rtl-source,
-    margin_left_rtl_source,
-    MarginLeftRTLSource,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_DIRECTIONAL_SOURCE |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "",
-    0,
-    kBoxPropSourceKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_None)
-#endif
-CSS_PROP_SHORTHAND(
    margin-right,
    margin_right,
    MarginRight,
-    CSS_PROPERTY_PARSE_FUNCTION |
+    CSS_PROPERTY_PARSE_VALUE |
        CSS_PROPERTY_UNITLESS_LENGTH_QUIRK |
+        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
+        CSS_PROPERTY_STORES_CALC |
        CSS_PROPERTY_APPLIES_TO_PAGE_RULE |
        CSS_PROPERTY_GETCS_NEEDS_LAYOUT_FLUSH,
-    "")
-#ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL
-CSS_PROP_MARGIN(
-    margin-right-value,
-    margin_right_value,
-    MarginRightValue,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_REPORT_OTHER_NAME |
-        CSS_PROPERTY_STORES_CALC |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
    "",
-    VARIANT_AHLP | VARIANT_CALC, // for internal use
+    VARIANT_AHLP | VARIANT_CALC,
    nullptr,
    offsetof(nsStyleMargin, mMargin),
    eStyleAnimType_Sides_Right)
-CSS_PROP_MARGIN(
-    margin-right-ltr-source,
-    margin_right_ltr_source,
-    MarginRightLTRSource,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_DIRECTIONAL_SOURCE |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "",
-    0,
-    kBoxPropSourceKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_None)
-CSS_PROP_MARGIN(
-    margin-right-rtl-source,
-    margin_right_rtl_source,
-    MarginRightRTLSource,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_DIRECTIONAL_SOURCE |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "",
-    0,
-    kBoxPropSourceKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_None)
-#endif
-CSS_PROP_SHORTHAND(
-    -moz-margin-start,
-    margin_start,
-    CSS_PROP_DOMPROP_PREFIXED(MarginStart),
-    CSS_PROPERTY_PARSE_FUNCTION |
-    CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "")
-#ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL
-CSS_PROP_MARGIN(
-    margin-start-value,
-    margin_start_value,
-    MarginStartValue,
-    CSS_PROPERTY_PARSE_INACCESSIBLE |
-        CSS_PROPERTY_APPLIES_TO_FIRST_LETTER |
-        CSS_PROPERTY_STORES_CALC |
-        CSS_PROPERTY_APPLIES_TO_PAGE_RULE,
-    "",
-    VARIANT_AHLP | VARIANT_CALC, // for internal use
-    nullptr,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_None)
-#endif
 CSS_PROP_MARGIN(
    margin-top,
    margin_top,
--- a/layout/style/nsCSSProps.cpp
+++ b/layout/style/nsCSSProps.cpp
@ -574,10 +574,6 @@ nsCSSProps::OtherNameFor(nsCSSProperty aProperty)
      return eCSSProperty_border_right_style;
    case eCSSProperty_border_right_width_value:
      return eCSSProperty_border_right_width;
-    case eCSSProperty_margin_left_value:
-      return eCSSProperty_margin_left;
-    case eCSSProperty_margin_right_value:
-      return eCSSProperty_margin_right;
    default:
      NS_ABORT_IF_FALSE(false, "bad caller");
  }
@ -2573,46 +2569,9 @@ static const nsCSSProperty gListStyleSubpropTable[] = {
 static const nsCSSProperty gMarginSubpropTable[] = {
  // Code relies on these being in top-right-bottom-left order.
  eCSSProperty_margin_top,
-  eCSSProperty_margin_right_value,
+  eCSSProperty_margin_right,
  eCSSProperty_margin_bottom,
-  eCSSProperty_margin_left_value,
-  // extras:
-  eCSSProperty_margin_left_ltr_source,
-  eCSSProperty_margin_left_rtl_source,
-  eCSSProperty_margin_right_ltr_source,
-  eCSSProperty_margin_right_rtl_source,
-  eCSSProperty_UNKNOWN
-};
-
-static const nsCSSProperty gMarginLeftSubpropTable[] = {
-  // nsCSSParser::ParseDirectionalBoxProperty depends on this order
-  eCSSProperty_margin_left_value,
-  eCSSProperty_margin_left_ltr_source,
-  eCSSProperty_margin_left_rtl_source,
-  eCSSProperty_UNKNOWN
-};
-
-static const nsCSSProperty gMarginRightSubpropTable[] = {
-  // nsCSSParser::ParseDirectionalBoxProperty depends on this order
-  eCSSProperty_margin_right_value,
-  eCSSProperty_margin_right_ltr_source,
-  eCSSProperty_margin_right_rtl_source,
-  eCSSProperty_UNKNOWN
-};
-
-static const nsCSSProperty gMarginStartSubpropTable[] = {
-  // nsCSSParser::ParseDirectionalBoxProperty depends on this order
-  eCSSProperty_margin_start_value,
-  eCSSProperty_margin_left_ltr_source,
-  eCSSProperty_margin_right_rtl_source,
-  eCSSProperty_UNKNOWN
-};
-
-static const nsCSSProperty gMarginEndSubpropTable[] = {
-  // nsCSSParser::ParseDirectionalBoxProperty depends on this order
-  eCSSProperty_margin_end_value,
-  eCSSProperty_margin_right_ltr_source,
-  eCSSProperty_margin_left_rtl_source,
+  eCSSProperty_margin_left,
  eCSSProperty_UNKNOWN
 };

--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@ -6502,28 +6502,13 @@ nsRuleNode::ComputeMarginData(void* aStartStruct,
 {
  COMPUTE_START_RESET(Margin, (), margin, parentMargin)

-  // margin: length, percent, auto, inherit
-  nsStyleCoord  coord;
-  nsCSSRect ourMargin;
-  ourMargin.mTop = *aRuleData->ValueForMarginTop();
-  ourMargin.mRight = *aRuleData->ValueForMarginRightValue();
-  ourMargin.mBottom = *aRuleData->ValueForMarginBottom();
-  ourMargin.mLeft = *aRuleData->ValueForMarginLeftValue();
-  AdjustLogicalBoxProp(aContext,
-                       *aRuleData->ValueForMarginLeftLTRSource(),
-                       *aRuleData->ValueForMarginLeftRTLSource(),
-                       *aRuleData->ValueForMarginStartValue(),
-                       *aRuleData->ValueForMarginEndValue(),
-                       NS_SIDE_LEFT, ourMargin, canStoreInRuleTree);
-  AdjustLogicalBoxProp(aContext,
-                       *aRuleData->ValueForMarginRightLTRSource(),
-                       *aRuleData->ValueForMarginRightRTLSource(),
-                       *aRuleData->ValueForMarginEndValue(),
-                       *aRuleData->ValueForMarginStartValue(),
-                       NS_SIDE_RIGHT, ourMargin, canStoreInRuleTree);
+  // margin: length, percent, calc, inherit
+  const nsCSSProperty* subprops =
+    nsCSSProps::SubpropertyEntryFor(eCSSProperty_margin);
+  nsStyleCoord coord;
  NS_FOR_CSS_SIDES(side) {
    nsStyleCoord parentCoord = parentMargin->mMargin.Get(side);
-    if (SetCoord(ourMargin.*(nsCSSRect::sides[side]),
+    if (SetCoord(*aRuleData->ValueFor(subprops[side]),
                 coord, parentCoord,
                 SETCOORD_LPAH | SETCOORD_INITIAL_ZERO | SETCOORD_STORE_CALC |
                   SETCOORD_UNSET_INITIAL,
--- a/layout/style/test/ListCSSProperties.cpp
+++ b/layout/style/test/ListCSSProperties.cpp
@ -132,14 +132,6 @@ const char *gInaccessibleProperties[] = {
    "border-start-color-value",
    "border-start-style-value",
    "border-start-width-value",
-    "margin-end-value",
-    "margin-left-value",
-    "margin-right-value",
-    "margin-start-value",
-    "margin-left-ltr-source",
-    "margin-left-rtl-source",
-    "margin-right-ltr-source",
-    "margin-right-rtl-source",
    "-moz-control-character-visibility",
    "-moz-script-level", // parsed by UA sheets only
    "-moz-script-size-multiplier",
--- a/layout/style/test/css_properties_like_longhand.js
+++ b/layout/style/test/css_properties_like_longhand.js
@ -1,7 +1,3 @@
 var gShorthandPropertiesLikeLonghand = [
-	{ name: "-moz-margin-end", prop: "MozMarginEnd"},
-	{ name: "margin-left", prop: "marginLeft"},
-	{ name: "margin-right", prop: "marginRight"},
-	{ name: "-moz-margin-start", prop: "MozMarginStart"},
 	{ name: "overflow", prop: "overflow"},
 ];
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@ -1432,7 +1432,8 @@ var gCSSProperties = {
  "-moz-margin-end": {
    domProp: "MozMarginEnd",
    inherited: false,
-    type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
+    type: CSS_TYPE_LONGHAND,
+    logical: true,
    get_computed: logical_box_prop_get_computed,
    /* no subproperties */
    /* auto may or may not be initial */
@ -1450,7 +1451,8 @@ var gCSSProperties = {
  "-moz-margin-start": {
    domProp: "MozMarginStart",
    inherited: false,
-    type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
+    type: CSS_TYPE_LONGHAND,
+    logical: true,
    get_computed: logical_box_prop_get_computed,
    /* no subproperties */
    /* auto may or may not be initial */
@ -2851,8 +2853,7 @@ var gCSSProperties = {
  "margin-left": {
    domProp: "marginLeft",
    inherited: false,
-    type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
-    /* no subproperties */
+    type: CSS_TYPE_LONGHAND,
    /* XXX testing auto has prerequisites */
    initial_values: [ "0", "0px", "0%", "calc(0pt)", "calc(0% + 0px)" ],
    other_values: [ "1px", "2em", "5%", ".5px", "+32px", "+.789px", "-.328px", "+0.56px", "-0.974px", "237px", "-289px", "-056px", "1987.45px", "-84.32px",
@ -2869,8 +2870,7 @@ var gCSSProperties = {
  "margin-right": {
    domProp: "marginRight",
    inherited: false,
-    type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
-    /* no subproperties */
+    type: CSS_TYPE_LONGHAND,
    /* XXX testing auto has prerequisites */
    initial_values: [ "0", "0px", "0%", "calc(0pt)", "calc(0% + 0px)" ],
    other_values: [ "1px", "2em", "5%",
@ -4612,6 +4612,42 @@ if (SpecialPowers.getBoolPref("layout.css.vertical-text.enabled")) {
                        "digits 12", "none 3", "digits 3.1415", "digits3", "digits 1",
                        "digits 3 all", "digits foo", "digits all", "digits 3.0" ]
    },
+    "margin-inline-end": {
+      domProp: "marginInlineEnd",
+      inherited: false,
+      type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
+      alias_for: "-moz-margin-end",
+      get_computed: logical_box_prop_get_computed,
+      /* XXX testing auto has prerequisites */
+      initial_values: [ "0", "0px", "0%", "calc(0pt)", "calc(0% + 0px)" ],
+      other_values: [ "1px", "2em", "5%",
+        "calc(2px)",
+        "calc(-2px)",
+        "calc(50%)",
+        "calc(3*25px)",
+        "calc(25px*3)",
+        "calc(3*25px + 50%)",
+      ],
+      invalid_values: [ "..25px", ".+5px", ".px", "-.px", "++5px", "-+4px", "+-3px", "--7px", "+-.6px", "-+.5px", "++.7px", "--.4px" ],
+    },
+    "margin-inline-start": {
+      domProp: "marginInlineStart",
+      inherited: false,
+      type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
+      alias_for: "-moz-margin-start",
+      get_computed: logical_box_prop_get_computed,
+      /* XXX testing auto has prerequisites */
+      initial_values: [ "0", "0px", "0%", "calc(0pt)", "calc(0% + 0px)" ],
+      other_values: [ "1px", "2em", "5%",
+        "calc(2px)",
+        "calc(-2px)",
+        "calc(50%)",
+        "calc(3*25px)",
+        "calc(25px*3)",
+        "calc(3*25px + 50%)",
+      ],
+      invalid_values: [ "..25px", ".+5px", ".px", "-.px", "++5px", "-+4px", "+-3px", "--7px", "+-.6px", "-+.5px", "++.7px", "--.4px" ],
+    },
    "padding-inline-end": {
      domProp: "paddingInlineEnd",
      inherited: false,
--- a/layout/style/test/test_page_parser.html
+++ b/layout/style/test/test_page_parser.html
@ -26,13 +26,9 @@
    // Check good properties with invalid units.
    { rule: _("margin: 2in; margin: 2vw;"), expected: {
      "margin-top": "2in",
-      "margin-right-value": "2in",
+      "margin-right": "2in",
      "margin-bottom": "2in",
-      "margin-left-value": "2in",
-      "margin-left-ltr-source": "physical",
-      "margin-left-rtl-source": "physical",
-      "margin-right-ltr-source": "physical",
-      "margin-right-rtl-source": "physical"
+      "margin-left": "2in"
    }},
    { rule: _("margin-top: 2in; margin-top: 2vw;"), expected: {"margin-top": "2in"}},
    { rule: _("margin-top: 2in; margin-top: 2vh;"), expected: {"margin-top": "2in"}},
@ -40,30 +36,16 @@
    { rule: _("margin-top: 2in; margin-top: 2vmin;"), expected: {"margin-top": "2in"}},

    // Check good properties.
-    // NOTE: The margin-*-value and margin-*-source properties are not really
-    // expected and will need to be removed once bug 241234 is addressed.
    { rule: _("margin: 2in;"), expected: {
      "margin-top": "2in",
-      "margin-right-value": "2in",
+      "margin-right": "2in",
      "margin-bottom": "2in",
-      "margin-left-value": "2in",
-      "margin-left-ltr-source": "physical",
-      "margin-left-rtl-source": "physical",
-      "margin-right-ltr-source": "physical",
-      "margin-right-rtl-source": "physical"
+      "margin-left": "2in"
    }},
    { rule: _("margin-top: 2in;"), expected: {"margin-top": "2in"}},
-    { rule: _("margin-left: 2in;"), expected: {
-      "margin-left-value": "2in",
-      "margin-left-ltr-source": "physical",
-      "margin-left-rtl-source": "physical",
-    }},
+    { rule: _("margin-left: 2in;"), expected: {"margin-left": "2in"}},
    { rule: _("margin-bottom: 2in;"), expected: {"margin-bottom": "2in"}},
-    { rule: _("margin-right: 2in;"), expected: {
-      "margin-right-value": "2in",
-      "margin-right-ltr-source": "physical",
-      "margin-right-rtl-source": "physical",
-    }}
+    { rule: _("margin-right: 2in;"), expected: {"margin-right": "2in"}}
  ];

  var display = document.getElementById("display");
--- a/layout/style/test/test_value_computation.html
+++ b/layout/style/test/test_value_computation.html
@ -13,7 +13,12 @@
  </style>
  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
  <script type="text/javascript">
-  SimpleTest.expectAssertions(9);
+  var numAssertions = 9;
+  if (SpecialPowers.getBoolPref("layout.css.vertical-text.enabled")) {
+    // these are from the additional margin-inline-{start,end} tests
+    numAssertions += 2;
+  }
+  SimpleTest.expectAssertions(numAssertions);
  SimpleTest.waitForExplicitFinish();
  SimpleTest.requestLongerTimeout(2);

@ -57,8 +62,8 @@ var gBadComputedNoFrame = {
  "border-bottom-right-radius": [ "0%", "calc(-1%)" ],
  "border-top-left-radius": [ "0%", "calc(-1%)" ],
  "border-top-right-radius": [ "0%", "calc(-1%)" ],
-  "-moz-margin-end": [ "0%", "calc(0% + 0px)", "calc(-1%)" ],
-  "-moz-margin-start": [ "0%", "calc(0% + 0px)", "calc(-1%)" ],
+  "-moz-margin-end": [ "0%", "calc(0% + 0px)" ],
+  "-moz-margin-start": [ "0%", "calc(0% + 0px)" ],
  "-moz-outline-radius": [ "0%", "calc(-1%)", "calc(0px) calc(0pt) calc(0%) calc(0em)" ],
  "-moz-outline-radius-bottomleft": [ "0%", "calc(-1%)" ],
  "-moz-outline-radius-bottomright": [ "0%", "calc(-1%)" ],
@ -68,6 +73,8 @@ var gBadComputedNoFrame = {
  "-moz-padding-start": [ "0%", "calc(0% + 0px)", "calc(-1%)" ],
  "margin": [ "0% 0px 0em 0pt" ],
  "margin-bottom": [ "0%", "calc(0% + 0px)" ],
+  "margin-inline-end": [ "0%", "calc(0% + 0px)" ],
+  "margin-inline-start": [ "0%", "calc(0% + 0px)" ],
  "margin-left": [ "0%", "calc(0% + 0px)" ],
  "margin-right": [ "0%", "calc(0% + 0px)" ],
  "margin-top": [ "0%", "calc(0% + 0px)" ],