Bug 1013160 - Remove padding suppressing mechanism of bullet. r=jfkthame,surkov

accessible/src/html/HTMLListAccessible.cpp

@@ -166,9 +166,6 @@ HTMLListBulletAccessible::Name(nsString &aName)
   nsBlockFrame* blockFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (blockFrame) {
     blockFrame->GetBulletText(aName);
-
-    // Append space otherwise bullets are jammed up against list text.
-    aName.Append(' ');
   }
 
   return eNameOK;

accessible/tests/mochitest/text/test_hypertext.html

@@ -71,8 +71,8 @@
       testText(IDs, 0, 1, kEmbedChar);
 
       IDs = [ "listitem" ];
-      testCharacterCount(IDs, 5);
-      testText(IDs, 0, 5, "1.foo");
+      testCharacterCount(IDs, 6);
+      testText(IDs, 0, 6, "1. foo");
 
       testText(["testbr"], 0, 3, "foo");
 

accessible/tests/mochitest/text/test_lineboundary.html

@@ -118,17 +118,17 @@
       // list items
 
       testTextAtOffset([ "li1" ], BOUNDARY_LINE_START,
-                       [ [ 0, 5, kDiscBulletChar + "Item", 0, 5 ] ]);
+                       [ [ 0, 6, kDiscBulletText + "Item", 0, 6 ] ]);
       testTextAtOffset([ "li2" ], BOUNDARY_LINE_START,
-                       [ [ 0, 1, kDiscBulletChar, 0, 1 ] ]);
+                       [ [ 0, 2, kDiscBulletText, 0, 2 ] ]);
       testTextAtOffset([ "li3" ], BOUNDARY_LINE_START,
-                       [ [ 0, 7, kDiscBulletChar + "a long ", 0, 8 ],
-                         [ 8, 11, "and ", 8, 12 ] ]);
+                       [ [ 0, 8, kDiscBulletText + "a long ", 0, 9 ],
+                         [ 9, 12, "and ", 9, 13 ] ]);
       testTextAtOffset([ "li4" ], BOUNDARY_LINE_START,
-                       [ [ 0, 6, kDiscBulletChar + "a " + kEmbedChar + " c", 0, 6 ] ]);
+                       [ [ 0, 7, kDiscBulletText + "a " + kEmbedChar + " c", 0, 7 ] ]);
       testTextAtOffset([ "li5" ], BOUNDARY_LINE_START,
-                       [ [ 0, 1, kDiscBulletChar + "\n", 0, 2 ],
-                         [ 2, 6, "hello", 2, 7 ] ]);
+                       [ [ 0, 2, kDiscBulletText + "\n", 0, 3 ],
+                         [ 3, 7, "hello", 3, 8 ] ]);
       testTextAtOffset([ "ul1" ], BOUNDARY_LINE_START,
                        [ [ 0, 0, kEmbedChar, 0, 1 ],
                          [ 1, 1, kEmbedChar, 1, 2 ],
@@ -137,17 +137,17 @@
                          [ 4, 5, kEmbedChar, 4, 5 ] ]);
 
       testTextAtOffset([ "li6" ], BOUNDARY_LINE_START,
-                       [ [ 0, 6, "1.Item", 0, 6 ] ]);
+                       [ [ 0, 7, "1. Item", 0, 7 ] ]);
       testTextAtOffset([ "li7" ], BOUNDARY_LINE_START,
-                       [ [ 0, 2, "2.", 0, 2 ] ]);
+                       [ [ 0, 3, "2. ", 0, 3 ] ]);
       testTextAtOffset([ "li8" ], BOUNDARY_LINE_START,
-                       [ [ 0, 8, "3.a long ", 0, 9 ],
-                         [ 9, 12, "and ", 9, 13 ] ]);
+                       [ [ 0, 9, "3. a long ", 0, 10 ],
+                         [ 10, 13, "and ", 10, 14 ] ]);
       testTextAtOffset([ "li9" ], BOUNDARY_LINE_START,
-                       [ [ 0, 7, "4.a " + kEmbedChar + " c", 0, 7 ] ]);
+                       [ [ 0, 8, "4. a " + kEmbedChar + " c", 0, 8 ] ]);
       testTextAtOffset([ "li10" ], BOUNDARY_LINE_START,
-                       [ [ 0, 2, "5.\n", 0, 3 ],
-                         [ 3, 7, "hello", 3, 8 ] ]);
+                       [ [ 0, 3, "5. \n", 0, 4 ],
+                         [ 4, 8, "hello", 4, 9 ] ]);
       testTextAtOffset([ "ol1" ], BOUNDARY_LINE_START,
                        [ [ 0, 0, kEmbedChar, 0, 1 ],
                          [ 1, 1, kEmbedChar, 1, 2 ],

accessible/tests/mochitest/textattrs/test_general.html

@@ -538,9 +538,9 @@
       var attrs = {
         "auto-generated": "true"
       };
-      testTextAttrs(ID, 0, attrs, defAttrs, 0, 2);
-      testTextAttrs(ID, 2, { }, defAttrs, 2, 6);
-      testTextAttrs(ID, 6, attrs, defAttrs, 6, 7);
+      testTextAttrs(ID, 0, attrs, defAttrs, 0, 3);
+      testTextAttrs(ID, 3, { }, defAttrs, 3, 7);
+      testTextAttrs(ID, 7, attrs, defAttrs, 7, 8);
 
        //////////////////////////////////////////////////////////////////////////
       // area19, "HTML5 mark tag" test

layout/generic/nsBlockFrame.cpp

@@ -6545,12 +6545,15 @@ nsBlockFrame::GetBulletText(nsAString& aText) const
   if (myList->GetListStyleImage() ||
       myList->mListStyleType == NS_STYLE_LIST_STYLE_DISC) {
     aText.Assign(kDiscCharacter);
+    aText.Append(' ');
   }
   else if (myList->mListStyleType == NS_STYLE_LIST_STYLE_CIRCLE) {
     aText.Assign(kCircleCharacter);
+    aText.Append(' ');
   }
   else if (myList->mListStyleType == NS_STYLE_LIST_STYLE_SQUARE) {
     aText.Assign(kSquareCharacter);
+    aText.Append(' ');
   }
   else if (myList->mListStyleType != NS_STYLE_LIST_STYLE_NONE) {
     nsBulletFrame* bullet = GetBullet();

layout/generic/nsBulletFrame.cpp

@@ -19,6 +19,7 @@
 #include "prprf.h"
 #include "nsDisplayList.h"
 #include "nsCounterManager.h"
+#include "nsBidiUtils.h"
 
 #include "imgIContainer.h"
 #include "imgRequestProxy.h"
@@ -312,8 +313,6 @@ nsBulletFrame::PaintBullet(nsRenderingContext& aRenderingContext, nsPoint aPt,
   nsRefPtr<nsFontMetrics> fm;
   aRenderingContext.SetColor(nsLayoutUtils::GetColor(this, eCSSProperty_color));
 
-  mTextIsRTL = false;
-
   nsAutoString text;
   switch (listStyleType) {
   case NS_STYLE_LIST_STYLE_NONE:
@@ -415,12 +414,15 @@ nsBulletFrame::PaintBullet(nsRenderingContext& aRenderingContext, nsPoint aPt,
     GetListItemText(*myList, text);
     aRenderingContext.SetFont(fm);
     nscoord ascent = fm->MaxAscent();
-    aRenderingContext.SetTextRunRTL(mTextIsRTL);
-    aRenderingContext.DrawString(
-        text, mPadding.left + aPt.x,
-        NSToCoordRound(nsLayoutUtils::GetSnappedBaselineY(
-                this, aRenderingContext.ThebesContext(),
-                mPadding.top + aPt.y, ascent)));
+    aPt.MoveBy(mPadding.left, mPadding.top);
+    aPt.y = NSToCoordRound(nsLayoutUtils::GetSnappedBaselineY(
+            this, aRenderingContext.ThebesContext(), aPt.y, ascent));
+    nsPresContext* presContext = PresContext();
+    if (!presContext->BidiEnabled() && HasRTLChars(text)) {
+      presContext->SetBidiEnabled();
+    }
+    nsLayoutUtils::DrawString(this, &aRenderingContext,
+                              text.get(), text.Length(), aPt);
     break;
   }
 }
@@ -1454,18 +1456,16 @@ nsBulletFrame::AppendCounterText(int32_t aListStyleType,
 
 /* static */ void
 nsBulletFrame::GetListItemSuffix(int32_t aListStyleType,
-                                 nsString& aResult,
-                                 bool& aSuppressPadding)
+                                 nsString& aResult)
 {
-  aResult = '.';
-  aSuppressPadding = false;
+  aResult.AssignLiteral(MOZ_UTF16(". "));
 
   switch (aListStyleType) {
     case NS_STYLE_LIST_STYLE_NONE: // used by counters code only
     case NS_STYLE_LIST_STYLE_DISC: // used by counters code only
     case NS_STYLE_LIST_STYLE_CIRCLE: // used by counters code only
     case NS_STYLE_LIST_STYLE_SQUARE: // used by counters code only
-      aResult.Truncate();
+      aResult = ' ';
       break;
 
     case NS_STYLE_LIST_STYLE_CJK_DECIMAL:
@@ -1485,7 +1485,6 @@ nsBulletFrame::GetListItemSuffix(int32_t aListStyleType,
     case NS_STYLE_LIST_STYLE_MOZ_CJK_HEAVENLY_STEM:
     case NS_STYLE_LIST_STYLE_MOZ_CJK_EARTHLY_BRANCH:
       aResult = 0x3001;
-      aSuppressPadding = true;
       break;
 
     case NS_STYLE_LIST_STYLE_KOREAN_HANGUL_FORMAL:
@@ -1493,7 +1492,7 @@ nsBulletFrame::GetListItemSuffix(int32_t aListStyleType,
     case NS_STYLE_LIST_STYLE_KOREAN_HANJA_FORMAL:
     case NS_STYLE_LIST_STYLE_MOZ_HANGUL:
     case NS_STYLE_LIST_STYLE_MOZ_HANGUL_CONSONANT:
-      aResult = ',';
+      aResult.AssignLiteral(MOZ_UTF16(", "));
       break;
   }
 }
@@ -1502,27 +1501,30 @@ void
 nsBulletFrame::GetListItemText(const nsStyleList& aListStyle,
                                nsString& result)
 {
-  const nsStyleVisibility* vis = StyleVisibility();
-
   NS_ASSERTION(aListStyle.mListStyleType != NS_STYLE_LIST_STYLE_NONE &&
                aListStyle.mListStyleType != NS_STYLE_LIST_STYLE_DISC &&
                aListStyle.mListStyleType != NS_STYLE_LIST_STYLE_CIRCLE &&
                aListStyle.mListStyleType != NS_STYLE_LIST_STYLE_SQUARE,
                "we should be using specialized code for these types");
 
-  result.Truncate();
-  AppendCounterText(aListStyle.mListStyleType, mOrdinal, result, mTextIsRTL);
+  bool isRTL;
+  nsAutoString number;
+  AppendCounterText(aListStyle.mListStyleType, mOrdinal, number, isRTL);
 
   nsAutoString suffix;
-  GetListItemSuffix(aListStyle.mListStyleType, suffix, mSuppressPadding);
+  GetListItemSuffix(aListStyle.mListStyleType, suffix);
 
-  // We're not going to do proper Bidi reordering on the list item marker, but
-  // just display the whole thing as RTL or LTR, so we fake reordering by
-  // appending the suffix to the end of the list item marker if the
-  // directionality of the characters is the same as the style direction or
-  // prepending it to the beginning if they are different.
-  result = (mTextIsRTL == (vis->mDirection == NS_STYLE_DIRECTION_RTL)) ?
-          result + suffix : suffix + result;
+  result.Truncate();
+  if (GetWritingMode().IsBidiLTR() != isRTL) {
+    result.Append(number);
+  } else {
+    // RLM = 0x200f, LRM = 0x200e
+    char16_t mark = isRTL ? 0x200f : 0x200e;
+    result.Append(mark);
+    result.Append(number);
+    result.Append(mark);
+  }
+  result.Append(suffix);
 }
 
 #define MIN_BULLET_SIZE 1
@@ -1579,14 +1581,26 @@ nsBulletFrame::GetDesiredSize(nsPresContext*  aCX,
 
     case NS_STYLE_LIST_STYLE_DISC:
     case NS_STYLE_LIST_STYLE_CIRCLE:
-    case NS_STYLE_LIST_STYLE_SQUARE:
+    case NS_STYLE_LIST_STYLE_SQUARE: {
       ascent = fm->MaxAscent();
       bulletSize = std::max(nsPresContext::CSSPixelsToAppUnits(MIN_BULLET_SIZE),
                           NSToCoordRound(0.8f * (float(ascent) / 2.0f)));
       mPadding.bottom = NSToCoordRound(float(ascent) / 8.0f);
       aMetrics.Width() = aMetrics.Height() = bulletSize;
       aMetrics.SetTopAscent(bulletSize + mPadding.bottom);
+
+      // Add spacing to the padding
+      nscoord halfEm = fm->EmHeight() / 2;
+      WritingMode wm = GetWritingMode();
+      if (wm.IsVertical()) {
+        mPadding.bottom += halfEm;
+      } else if (wm.IsBidiLTR()) {
+        mPadding.right += halfEm;
+      } else {
+        mPadding.left += halfEm;
+      }
       break;
+    }
 
     default:
     case NS_STYLE_LIST_STYLE_DECIMAL_LEADING_ZERO:
@@ -1669,22 +1683,15 @@ nsBulletFrame::Reflow(nsPresContext* aPresContext,
   SetFontSizeInflation(inflation);
 
   // Get the base size
-  // This will also set mSuppressPadding appropriately (via GetListItemText())
-  // for the builtin counter styles with ideographic comma as suffix where the
-  // default padding from ua.css is not desired.
   GetDesiredSize(aPresContext, aReflowState.rendContext, aMetrics, inflation);
 
   // Add in the border and padding; split the top/bottom between the
   // ascent and descent to make things look nice
   const nsMargin& borderPadding = aReflowState.ComputedPhysicalBorderPadding();
-  if (!mSuppressPadding ||
-      aPresContext->HasAuthorSpecifiedRules(this,
-                                            NS_AUTHOR_SPECIFIED_PADDING)) {
-    mPadding.top += NSToCoordRound(borderPadding.top * inflation);
-    mPadding.right += NSToCoordRound(borderPadding.right * inflation);
-    mPadding.bottom += NSToCoordRound(borderPadding.bottom * inflation);
-    mPadding.left += NSToCoordRound(borderPadding.left * inflation);
-  }
+  mPadding.top += NSToCoordRound(borderPadding.top * inflation);
+  mPadding.right += NSToCoordRound(borderPadding.right * inflation);
+  mPadding.bottom += NSToCoordRound(borderPadding.bottom * inflation);
+  mPadding.left += NSToCoordRound(borderPadding.left * inflation);
   aMetrics.Width() += mPadding.left + mPadding.right;
   aMetrics.Height() += mPadding.top + mPadding.bottom;
   aMetrics.SetTopAscent(aMetrics.TopAscent() + mPadding.top);

layout/generic/nsBulletFrame.h

@@ -85,8 +85,7 @@ public:
 
   /* get suffix of list item */
   static void GetListItemSuffix(int32_t aListStyleType,
-                                nsString& aResult,
-                                bool& aSuppressPadding);
+                                nsString& aResult);
 
   /* get list item text, with '.' */
   void GetListItemText(const nsStyleList& aStyleList, nsString& aResult);
@@ -124,13 +123,6 @@ protected:
 
   nsSize mIntrinsicSize;
   int32_t mOrdinal;
-  bool mTextIsRTL;
-
-  // If set to true, any padding of bullet defined in the UA style sheet will
-  // be suppressed.  This is used for some CJK numbering styles where extra
-  // space after the suffix is not desired.  Note that, any author-specified
-  // padding overriding the default style will NOT be suppressed.
-  bool mSuppressPadding;
 
 private:
 

layout/reftests/counters/counter-suffix-ref.html

@@ -8,56 +8,30 @@
     text-align: end;
     box-sizing: border-box;
   }
-  .def span { -moz-padding-end: 0.5em; }
-  .fix span { -moz-padding-end: 1em; }
 </style>
 <div id="wrapper">
-  <p class="def">
-    <span>1.</span>foo<br>
-    <span>2.</span>bar
+  <p>
+    <span>1.&nbsp;</span>foo<br>
+    <span>2.&nbsp;</span>bar
   </p>
-  <p class="def">
-    <span>&#x5d0;.</span>foo<br>
-    <span>&#x5d1;.</span>bar
+  <p>
+    <span>&#x5d0;.&nbsp;</span>foo<br>
+    <span>&#x5d1;.&nbsp;</span>bar
   </p>
   <p>
     <span>&#x4e00;&#x3001;</span>foo<br>
     <span>&#x4e8c;&#x3001;</span>bar
   </p>
-  <p class="def">
-    <span>&#xc77c;,</span>foo<br>
-    <span>&#xc774;,</span>bar
+  <p>
+    <span>&#xc77c;,&nbsp;</span>foo<br>
+    <span>&#xc774;,&nbsp;</span>bar
   </p>
-  <p class="fix">
-    <span>1.</span>foo<br>
-    <span>2.</span>bar
+  <p dir="rtl">
+    <span>1.&nbsp;</span>foo<br>
+    <span>2.&nbsp;</span>bar
   </p>
-  <p class="fix">
-    <span>&#x5d0;.</span>foo<br>
-    <span>&#x5d1;.</span>bar
-  </p>
-  <p class="fix">
-    <span>&#x4e00;&#x3001;</span>foo<br>
-    <span>&#x4e8c;&#x3001;</span>bar
-  </p>
-  <p class="fix">
-    <span>&#xc77c;,</span>foo<br>
-    <span>&#xc774;,</span>bar
-  </p>
-  <p class="def" dir="rtl">
-    <span>1.</span>foo<br>
-    <span>2.</span>bar
-  </p>
-  <p class="def" dir="rtl">
-    <span>&#x5d0;.</span>foo<br>
-    <span>&#x5d1;.</span>bar
-  </p>
-  <p class="fix" dir="rtl">
-    <span>1.</span>foo<br>
-    <span>2.</span>bar
-  </p>
-  <p class="fix" dir="rtl">
-    <span>&#x5d0;.</span>foo<br>
-    <span>&#x5d1;.</span>bar
+  <p dir="rtl">
+    <span>&#x5d0;.&nbsp;</span>foo<br>
+    <span>&#x5d1;.&nbsp;</span>bar
   </p>
 </div>

layout/reftests/counters/counter-suffix.html

@@ -7,21 +7,12 @@
   .heb { list-style-type: hebrew; }
   .cjk { list-style-type: cjk-decimal; }
   .kor { list-style-type: korean-hangul-formal; }
-  .spec li::-moz-list-number {
-    -moz-padding-end: 1em;
-  }
 </style>
 <div id="wrapper">
   <ol class="dec"><li>foo<li>bar</ol>
   <ol class="heb"><li>foo<li>bar</ol>
   <ol class="cjk"><li>foo<li>bar</ol>
   <ol class="kor"><li>foo<li>bar</ol>
-  <ol class="dec spec"><li>foo<li>bar</ol>
-  <ol class="heb spec"><li>foo<li>bar</ol>
-  <ol class="cjk spec"><li>foo<li>bar</ol>
-  <ol class="kor spec"><li>foo<li>bar</ol>
   <ol class="dec" dir="rtl"><li>foo<li>bar</ol>
   <ol class="heb" dir="rtl"><li>foo<li>bar</ol>
-  <ol class="dec spec" dir="rtl"><li>foo<li>bar</ol>
-  <ol class="heb spec" dir="rtl"><li>foo<li>bar</ol>
 </div>

layout/reftests/counters/reftest.list

@@ -68,7 +68,7 @@ fails-if(B2G&&browserIsRemote) == t1204-reset-02-c-o-test.html t1204-reset-02-c-
 == counter-ua-limits-list-00.html counter-ua-limits-list-00-ref.html
 == counter-ua-limits-list-01.html counter-ua-limits-list-01-ref.html
 == multiple-thai-counters.html multiple-thai-counters-ref.html
-fails-if(B2G) == counter-suffix.html counter-suffix-ref.html # B2G kerning
+== counter-suffix.html counter-suffix-ref.html
 == counter-cjk-decimal.html counter-cjk-decimal-ref.html
 == counter-japanese-informal.html counter-japanese-informal-ref.html
 == counter-japanese-formal.html counter-japanese-formal-ref.html

layout/reftests/list-item/numbering-2-ref.html

@@ -6,7 +6,7 @@ html,body {
     color:black; background-color:white; font-size:12px; padding:0; margin:0;
 }
 
-li {margin-left:0; padding-left:0px; }
+li {margin-left:0; padding-left:0px; width:200px; }
 ol {margin-left:0; padding-left:40px; }
 </style>
 </head>

layout/reftests/list-item/numbering-2.html

@@ -6,7 +6,7 @@ html,body {
     color:black; background-color:white; font-size:12px; padding:0; margin:0;
 }
 
-li {margin-left:0; padding-left:0px; }
+li {margin-left:0; padding-left:0px; width:200px; }
 ol {margin-left:0; padding-left:40px; }
 </style>
 </head>

layout/style/ua.css

@@ -77,9 +77,6 @@
 *|*::-moz-list-bullet, *|*::-moz-list-number {
   display: inline;
   vertical-align: baseline;
-  /* Note that this padding is suppressed for some CJK numbering styles;
-   * see bug 934072 */
-  -moz-padding-end: 0.5em;
 }
 
 /* Links */