Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Issue 985923002: Revert of adding baseline options for super/sub scripting (Closed)

Created:
5 years, 9 months ago by dschuyler
Modified:
5 years, 9 months ago
Reviewers:
msw, dgrogan
CC:
chromium-reviews, groby-ooo-7-16, Peter Kasting
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of adding baseline options for super/sub scripting (patchset #20 id:420001 of https://codereview.chromium.org/924543004/) Reason for revert: Windows XP appears to be drawing outside of the expected draw area for a font. The short term fix will be to disable the test for Windows XP and put in a TODO to look into it. Original issue's description: > adding baseline options for super/sub scripting > > This change list adds four options for a smaller font to be used as a style within the render text. > > Option Example > ------ ------- > SUPERSCRIPT a mathematical exponent would be superscript > SUPERIOR 8th, the "th" would be superior script > INFERIOR 1/2, the "2" would be inferior ("1" is superior) > SUBSCRIPT H2O, the "2" would be subscript > > Some imagination is needed to interpret the examples above. To see a clearer references to what is meant, this wikipedia article may be helpful: http://en.wikipedia.org/wiki/Subscript_and_superscript > > These options may be set in the same way options like BOLD and ITALIC can currently be set, e.g. with a call to my_render_text->ApplyStyle(gfx::SUPERSCRIPT, true, gfx::Range(1, 3)); > > BUG=459812 > > Committed: https://crrev.com/eefeef59e4b44727e735e99733799a489b8a6efa > Cr-Commit-Position: refs/heads/master@{#319492} TBR=mukai@chromium.org,dgrogan@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=459812

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -299 lines) Patch
M ui/gfx/render_text.h View 5 chunks +3 lines, -14 lines 0 comments Download
M ui/gfx/render_text.cc View 9 chunks +22 lines, -43 lines 0 comments Download
M ui/gfx/render_text_harfbuzz.h View 1 chunk +0 lines, -2 lines 0 comments Download
M ui/gfx/render_text_harfbuzz.cc View 6 chunks +5 lines, -36 lines 0 comments Download
M ui/gfx/render_text_mac.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/render_text_unittest.cc View 5 chunks +21 lines, -184 lines 0 comments Download
M ui/gfx/text_constants.h View 1 chunk +0 lines, -19 lines 0 comments Download

Messages

Total messages: 12 (2 generated)
dschuyler
Created Revert of adding baseline options for super/sub scripting
5 years, 9 months ago (2015-03-07 00:44:53 UTC) #1
dschuyler
5 years, 9 months ago (2015-03-07 00:45:45 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/985923002/1
5 years, 9 months ago (2015-03-07 00:45:53 UTC) #4
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 9 months ago (2015-03-07 00:45:55 UTC) #6
Jun Mukai
On 2015/03/07 at 00:45:55, commit-bot wrote: > No LGTM from a valid reviewer yet. Only ...
5 years, 9 months ago (2015-03-07 01:12:49 UTC) #7
chromium-reviews
I'm testing a fix in https://codereview.chromium.org/988763005/ So if that's done soon, maybe that will help. ...
5 years, 9 months ago (2015-03-07 01:14:03 UTC) #8
dschuyler
On 2015/03/07 01:14:03, chromium-reviews wrote: > I'm testing a fix in https://codereview.chromium.org/988763005/ > So if ...
5 years, 9 months ago (2015-03-07 02:17:44 UTC) #9
dschuyler
On 2015/03/07 02:17:44, dschuyler wrote: > On 2015/03/07 01:14:03, chromium-reviews wrote: > > I'm testing ...
5 years, 9 months ago (2015-03-07 02:22:09 UTC) #10
Jun Mukai
On 2015/03/07 02:22:09, dschuyler wrote: > On 2015/03/07 02:17:44, dschuyler wrote: > > On 2015/03/07 ...
5 years, 9 months ago (2015-03-07 02:31:32 UTC) #11
chromium-reviews
5 years, 9 months ago (2015-03-07 02:37:35 UTC) #12
I'm quite new, so I'll need an LGTM to commit the revert (I believe).

On Fri, Mar 6, 2015 at 6:31 PM, <mukai@chromium.org> wrote:

> On 2015/03/07 02:22:09, dschuyler wrote:
>
>> On 2015/03/07 02:17:44, dschuyler wrote:
>> > On 2015/03/07 01:14:03, chromium-reviews wrote:
>> > > I'm testing a fix in https://codereview.chromium.org/988763005/
>> > > So if that's done soon, maybe that will help.
>> > >
>> > > On Fri, Mar 6, 2015 at 5:12 PM, <mailto:mukai@chromium.org> wrote:
>> > >
>> > > > On 2015/03/07 at 00:45:55, commit-bot wrote:
>> > > >
>> > > >> No LGTM from a valid reviewer yet. Only full committers are
>> accepted.
>> > > >> Even if an LGTM may have been provided, it was from a
>> non-committer,
>> > > >> _not_ a full super star committer.
>> > > >> See http://www.chromium.org/getting-involved/become-a-committer
>> > > >> Note that this has nothing to do with OWNERS files.
>> > > >>
>> > > >
>> > > > Be careful this will conflict with my CL
>> > > > https://codereview.chromium.org/968923004
>> > > > I already rebased my CL and now it's in CQ.
>> > > > What should I do?  Should I wait for your revert or can I make a
>>
> progress?
>
>> > > >
>> > > > https://codereview.chromium.org/985923002/
>> > > >
>> > >
>> > > To unsubscribe from this group and stop receiving emails from it,
>> send an
>> > email
>> > > to mailto:chromium-reviews+unsubscribe@chromium.org.
>> >
>> > With the LGTM from MSW on https://codereview.chromium.org/988763005/, I
>>
> think
>
>> it
>> > would be correct to  not check this in.
>>
>
>  With the NOT LGTM from MUKAI on https://codereview.chromium.
>> org/988763005/, it
>> sounds like this revert should proceed.
>>
>
> I believe so.  If you don't have a committer access, I can revert on
> behalf.
>
> https://codereview.chromium.org/985923002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698