Skip to content

GitLab

T
tangled
Commit a3838bf2 authored by mpearson's avatar mpearson Committed by Commit bot
Browse files
Options

Revert of grit: Automatically replace ... with … (patchset #10 id:180001 of... 

Revert of grit: Automatically replace ... with … (patchset #10 id:180001 of https://codereview.chromium.org/2112653003/ )

Reason for revert:
Likely cause of failures:

---
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/67858

unexpected_failures:
fast/forms/color/color-suggestion-picker-appearance-zoom125.html
fast/forms/color/color-suggestion-picker-with-scrollbar-appearance.html
fast/forms/color/color-suggestion-picker-appearance-zoom200.html

all with the message
image diff
---

since it appears to replace "..." with a proper ellipsis, which all the three
tests show differences from:

https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux/67858/layout-test-results/results.html

also failures for

also some other bots for tests
fast/forms/color/color-suggestion-picker-appearance.html
fast/forms/color/color-suggestion-picker-one-row-appearance.html
fast/forms/color/color-suggestion-picker-two-row-appearance.html
appear on other bots.

And, for what it's worth,
the bot
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/3384
started showing a renderer crash in
http/tests/images/restyle-decode-error.html
in a blamelist that includes this change as well.  Hopefully this revert will fix that too. :-)

Original issue's description:
> grit: Automatically replace ... with … (U+2026)
>
> Android lint tool pointed out that ... should really be … (U+2026).
> Tested that this glyph renders well on Mac + Win as well, so
> applying to all OS's by default.
>
> TBR=flackr
> BUG=621772
>
> Committed: https://crrev.com/e82c4e1b9de3b189c85bccff63d4ef3046e44d43
> Cr-Commit-Position: refs/heads/master@{#407182}

TBR=flackr@chromium.org,asanka@chromium.org,agrieve@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=621772

Review-Url: https://codereview.chromium.org/2176663003
Cr-Commit-Position: refs/heads/master@{#407219}
parent ab57275f
Hide whitespace changes
Inline Side-by-side
Showing with 14 additions and 52 deletions
chrome/browser/download/download_item_model_unittest.cc
View file @ a3838bf2
...@@ -308,9 +308,9 @@ TEST_F(DownloadItemModelTest, InProgressStatus) { ...@@ -308,9 +308,9 @@ TEST_F(DownloadItemModelTest, InProgressStatus) {
// .-- .TimeRemaining() is known. // .-- .TimeRemaining() is known.
// | .-- .GetOpenWhenComplete() // | .-- .GetOpenWhenComplete()
// | | .---- .IsPaused() // | | .---- .IsPaused()
{ 0, 0, false, false, false, "Starting\xE2\x80\xA6" }, { 0, 0, false, false, false, "Starting..." },
{ 1, 0, false, false, false, "1 B" }, { 1, 0, false, false, false, "1 B" },
{ 0, 2, false, false, false, "Starting\xE2\x80\xA6"}, { 0, 2, false, false, false, "Starting..." },
{ 1, 2, false, false, false, "1/2 B" }, { 1, 2, false, false, false, "1/2 B" },
{ 0, 2, true, false, false, "0/2 B, 10 secs left" }, { 0, 2, true, false, false, "0/2 B, 10 secs left" },
{ 1, 2, true, false, false, "1/2 B, 10 secs left" }, { 1, 2, true, false, false, "1/2 B, 10 secs left" },
...@@ -318,8 +318,8 @@ TEST_F(DownloadItemModelTest, InProgressStatus) { ...@@ -318,8 +318,8 @@ TEST_F(DownloadItemModelTest, InProgressStatus) {
{ 1, 0, false, true, false, "Opening when complete" }, { 1, 0, false, true, false, "Opening when complete" },
{ 0, 2, false, true, false, "Opening when complete" }, { 0, 2, false, true, false, "Opening when complete" },
{ 1, 2, false, true, false, "Opening when complete" }, { 1, 2, false, true, false, "Opening when complete" },
{ 0, 2, true, true, false, "Opening in 10 secs\xE2\x80\xA6"}, { 0, 2, true, true, false, "Opening in 10 secs..." },
{ 1, 2, true, true, false, "Opening in 10 secs\xE2\x80\xA6"}, { 1, 2, true, true, false, "Opening in 10 secs..." },
{ 0, 0, false, false, true, "0 B, Paused" }, { 0, 0, false, false, true, "0 B, Paused" },
{ 1, 0, false, false, true, "1 B, Paused" }, { 1, 0, false, false, true, "1 B, Paused" },
{ 0, 2, false, false, true, "0/2 B, Paused" }, { 0, 2, false, false, true, "0/2 B, Paused" },
......
tools/grit/grit/node/message.py
View file @ a3838bf2
...@@ -11,16 +11,15 @@ import types ...@@ -11,16 +11,15 @@ import types
from grit.node import base from grit.node import base
import grit.format.rc_header
import grit.format.rc
from grit import clique from grit import clique
from grit import exception from grit import exception
from grit import lazy_re from grit import lazy_re
from grit import tclib from grit import tclib
from grit import util from grit import util
# Matches exactly three dots ending a line or followed by whitespace.
_ELLIPSIS_PATTERN = lazy_re.compile(r'(?<!\.)\.\.\.(?=$|\s)')
_ELLIPSIS_SYMBOL = u'\u2026' # Ellipsis
# Finds whitespace at the start and end of a string which can be multiline. # Finds whitespace at the start and end of a string which can be multiline.
_WHITESPACE = lazy_re.compile('(?P<start>\s*)(?P<body>.+?)(?P<end>\s*)\Z', _WHITESPACE = lazy_re.compile('(?P<start>\s*)(?P<body>.+?)(?P<end>\s*)\Z',
re.DOTALL | re.MULTILINE) re.DOTALL | re.MULTILINE)
...@@ -55,9 +54,6 @@ class MessageNode(base.ContentNode): ...@@ -55,9 +54,6 @@ class MessageNode(base.ContentNode):
# Example: "foo=5 bar baz=100" # Example: "foo=5 bar baz=100"
self.formatter_data = {} self.formatter_data = {}
# Whether or not to convert ... -> U+2026 within Translate().
self._replace_ellipsis = False
def _IsValidChild(self, child): def _IsValidChild(self, child):
return isinstance(child, (PhNode)) return isinstance(child, (PhNode))
...@@ -72,11 +68,6 @@ class MessageNode(base.ContentNode): ...@@ -72,11 +68,6 @@ class MessageNode(base.ContentNode):
return False return False
return True return True
def SetReplaceEllipsis(self, value):
'''Sets whether to replace ... with \u2026.
'''
self._replace_ellipsis = value
def MandatoryAttributes(self): def MandatoryAttributes(self):
return ['name|offset'] return ['name|offset']
...@@ -209,8 +200,6 @@ class MessageNode(base.ContentNode): ...@@ -209,8 +200,6 @@ class MessageNode(base.ContentNode):
self.PseudoIsAllowed(), self.PseudoIsAllowed(),
self.ShouldFallbackToEnglish() self.ShouldFallbackToEnglish()
).GetRealContent() ).GetRealContent()
if self._replace_ellipsis:
msg = _ELLIPSIS_PATTERN.sub(_ELLIPSIS_SYMBOL, msg)
return msg.replace('[GRITLANGCODE]', lang) return msg.replace('[GRITLANGCODE]', lang)
def NameOrOffset(self): def NameOrOffset(self):
......
tools/grit/grit/node/message_unittest.py
View file @ a3838bf2
...@@ -8,16 +8,12 @@ ...@@ -8,16 +8,12 @@
import os import os
import sys import sys
if __name__ == '__main__':
sys.path.append(os.path.join(os.path.dirname(__file__), '../..'))
import unittest import unittest
import StringIO import StringIO
if __name__ == '__main__':
# When executed as the main module, the first entry in sys.path will be
# the directory contain this module. This entry causes the io.py file in this
# directory to be selected whenever any file does "import io", rather than the
# system "io" module. As a work-around, remove the first sys.path entry.
sys.path[0] = os.path.join(os.path.dirname(__file__), '../..')
from grit import tclib from grit import tclib
from grit import util from grit import util
from grit.node import message from grit.node import message
...@@ -89,18 +85,6 @@ class MessageUnittest(unittest.TestCase): ...@@ -89,18 +85,6 @@ class MessageUnittest(unittest.TestCase):
self.failUnlessEqual(expected_formatter_data[key], self.failUnlessEqual(expected_formatter_data[key],
msg.formatter_data[key]) msg.formatter_data[key])
def testReplaceEllipsis(self):
root = util.ParseGrdForUnittest('''
<messages>
<message name="IDS_GREETING" desc="">
A...B.... <ph name="PH">%s<ex>A</ex></ph>... B... C...
</message>
</messages>''')
msg, = root.GetChildrenOfType(message.MessageNode)
msg.SetReplaceEllipsis(True)
content = msg.Translate('en')
self.failUnlessEqual(u'A...B.... %s\u2026 B\u2026 C\u2026', content)
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()
tools/grit/grit/tool/build.py
View file @ a3838bf2
...@@ -15,12 +15,9 @@ import shutil ...@@ -15,12 +15,9 @@ import shutil
import sys import sys
from grit import grd_reader from grit import grd_reader
from grit import shortcuts
from grit import util from grit import util
from grit.node import include
from grit.node import message
from grit.node import structure
from grit.tool import interface from grit.tool import interface
from grit import shortcuts
# It would be cleaner to have each module register itself, but that would # It would be cleaner to have each module register itself, but that would
...@@ -148,12 +145,10 @@ are exported to translation interchange files (e.g. XMB files), etc. ...@@ -148,12 +145,10 @@ are exported to translation interchange files (e.g. XMB files), etc.
output_all_resource_defines = None output_all_resource_defines = None
write_only_new = False write_only_new = False
depend_on_stamp = False depend_on_stamp = False
replace_ellipsis = True
(own_opts, args) = getopt.getopt(args, 'a:o:D:E:f:w:t:h:', (own_opts, args) = getopt.getopt(args, 'a:o:D:E:f:w:t:h:',
('depdir=','depfile=','assert-file-list=', ('depdir=','depfile=','assert-file-list=',
'output-all-resource-defines', 'output-all-resource-defines',
'no-output-all-resource-defines', 'no-output-all-resource-defines',
'no-replace-ellipsis',
'depend-on-stamp', 'depend-on-stamp',
'write-only-new=')) 'write-only-new='))
for (key, val) in own_opts: for (key, val) in own_opts:
...@@ -181,8 +176,6 @@ are exported to translation interchange files (e.g. XMB files), etc. ...@@ -181,8 +176,6 @@ are exported to translation interchange files (e.g. XMB files), etc.
output_all_resource_defines = True output_all_resource_defines = True
elif key == '--no-output-all-resource-defines': elif key == '--no-output-all-resource-defines':
output_all_resource_defines = False output_all_resource_defines = False
elif key == '--no-replace-ellipsis':
replace_ellipsis = False
elif key == '-t': elif key == '-t':
target_platform = val target_platform = val
elif key == '-h': elif key == '-h':
...@@ -234,13 +227,6 @@ are exported to translation interchange files (e.g. XMB files), etc. ...@@ -234,13 +227,6 @@ are exported to translation interchange files (e.g. XMB files), etc.
if rc_header_format: if rc_header_format:
self.res.AssignRcHeaderFormat(rc_header_format) self.res.AssignRcHeaderFormat(rc_header_format)
self.res.RunGatherers() self.res.RunGatherers()
# Replace ... with the single-character version. http://crbug.com/621772
if replace_ellipsis:
for node in self.res:
if isinstance(node, message.MessageNode):
node.SetReplaceEllipsis(True)
self.Process() self.Process()
if assert_output_files: if assert_output_files:
...@@ -281,6 +267,9 @@ are exported to translation interchange files (e.g. XMB files), etc. ...@@ -281,6 +267,9 @@ are exported to translation interchange files (e.g. XMB files), etc.
def AddWhitelistTags(start_node, whitelist_names): def AddWhitelistTags(start_node, whitelist_names):
# Walk the tree of nodes added attributes for the nodes that shouldn't # Walk the tree of nodes added attributes for the nodes that shouldn't
# be written into the target files (skip markers). # be written into the target files (skip markers).
from grit.node import include
from grit.node import message
from grit.node import structure
for node in start_node: for node in start_node:
# Same trick data_pack.py uses to see what nodes actually result in # Same trick data_pack.py uses to see what nodes actually result in
# real items. # real items.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment