Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
7989002:
Revert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds. (Closed)
|
Created:
9 years, 3 months ago by tommi (sloooow) - chröme Modified:
9 years, 3 months ago CC:
chromium-reviews, darin-cc_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src/ Visibility:
Public. |
DescriptionRevert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds.
BUG=none
TEST=manually
Review URL: http://codereview.chromium.org/7983043
TBR=fsamuel@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102266
Patch Set 1 #
Messages
Total messages: 5 (0 generated)
FYI - the reason for the revert is that your change broke the ClickingMovesFocus unit test on Linux Tests (Views Dbg) 1. Please take a look before committing again. On Thu, Sep 22, 2011 at 1:40 PM, <tommi@chromium.org> wrote: > Reviewers: Fady Samuel, > > Description: > Revert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds. > > BUG=none > TEST=manually > > > Review URL: http://codereview.chromium.**org/7983043<http://codereview.chromium.org/7983043> > > TBR=fsamuel@chromium.org > > Please review this at http://codereview.chromium.**org/7989002/<http://codereview.chromium.org/7989... > > SVN Base: svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/> > > Affected files: > M webkit/glue/webkit_resources.**grd > M webkit/glue/**webkitplatformsupport_impl.cc > > > Index: webkit/glue/webkit_resources.**grd > ==============================**==============================**======= > --- webkit/glue/webkit_resources.**grd (revision 102265) > +++ webkit/glue/webkit_resources.**grd (working copy) > @@ -52,7 +52,7 @@ > <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB" > file="resources\mediaplayer_**volume_slider_thumb.png" type="BINDATA" /> > <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_HOVER" > file="resources\mediaplayer_**volume_slider_thumb_hover.png" > type="BINDATA" /> > <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_DOWN" > file="resources\mediaplayer_**volume_slider_thumb_down.png" type="BINDATA" > /> > - <if expr="pp_ifdef('touchui') or is_macosx"> > + <if expr="is_macosx"> > <include name="IDR_OVERHANG_PATTERN" file="resources\overhang_**pattern.png" > type="BINDATA" /> > </if> > <include name="IDC_PAN_EAST" file="resources\pan_east.cur" > type="CURSOR" /> > Index: webkit/glue/**webkitplatformsupport_impl.cc > ==============================**==============================**======= > --- webkit/glue/**webkitplatformsupport_impl.cc (revision 102265) > +++ webkit/glue/**webkitplatformsupport_impl.cc (working copy) > @@ -394,7 +394,7 @@ > IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_HOVER }, > { "**mediaplayerVolumeSliderThumbDo**wn", > IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_DOWN }, > -#if defined(OS_MACOSX) || defined(TOUCH_UI) > +#if defined(OS_MACOSX) > { "overhangPattern", IDR_OVERHANG_PATTERN }, > #endif > { "panIcon", IDR_PAN_SCROLL_ICON }, > > >
This seems very, very odd given the patch to actually uses the resource in WebKit hasn't even been upstreamed yet... On Thu, Sep 22, 2011 at 7:55 AM, Tommi <tommi@chromium.org> wrote: > FYI - the reason for the revert is that your change broke > the ClickingMovesFocus unit test on Linux Tests (Views Dbg) 1. > Please take a look before committing again. > > > On Thu, Sep 22, 2011 at 1:40 PM, <tommi@chromium.org> wrote: > >> Reviewers: Fady Samuel, >> >> Description: >> Revert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds. >> >> BUG=none >> TEST=manually >> >> >> Review URL: http://codereview.chromium.**org/7983043<http://codereview.chromium.org/7983043> >> >> TBR=fsamuel@chromium.org >> >> Please review this at http://codereview.chromium.**org/7989002/<http://codereview.chromium.org/7989... >> >> SVN Base: svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/> >> >> Affected files: >> M webkit/glue/webkit_resources.**grd >> M webkit/glue/**webkitplatformsupport_impl.cc >> >> >> Index: webkit/glue/webkit_resources.**grd >> ==============================**==============================**======= >> --- webkit/glue/webkit_resources.**grd (revision 102265) >> +++ webkit/glue/webkit_resources.**grd (working copy) >> @@ -52,7 +52,7 @@ >> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB" >> file="resources\mediaplayer_**volume_slider_thumb.png" type="BINDATA" /> >> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_HOVER" >> file="resources\mediaplayer_**volume_slider_thumb_hover.png" >> type="BINDATA" /> >> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_DOWN" >> file="resources\mediaplayer_**volume_slider_thumb_down.png" >> type="BINDATA" /> >> - <if expr="pp_ifdef('touchui') or is_macosx"> >> + <if expr="is_macosx"> >> <include name="IDR_OVERHANG_PATTERN" file="resources\overhang_**pattern.png" >> type="BINDATA" /> >> </if> >> <include name="IDC_PAN_EAST" file="resources\pan_east.cur" >> type="CURSOR" /> >> Index: webkit/glue/**webkitplatformsupport_impl.cc >> ==============================**==============================**======= >> --- webkit/glue/**webkitplatformsupport_impl.cc (revision 102265) >> +++ webkit/glue/**webkitplatformsupport_impl.cc (working copy) >> @@ -394,7 +394,7 @@ >> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_HOVER }, >> { "**mediaplayerVolumeSliderThumbDo**wn", >> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_DOWN }, >> -#if defined(OS_MACOSX) || defined(TOUCH_UI) >> +#if defined(OS_MACOSX) >> { "overhangPattern", IDR_OVERHANG_PATTERN }, >> #endif >> { "panIcon", IDR_PAN_SCROLL_ICON }, >> >> >> >
Yes, sorry, I reverted my revert again. The builder passed once and only once, so your change is back in. On Thu, Sep 22, 2011 at 3:48 PM, Fady Samuel <fsamuel@chromium.org> wrote: > This seems very, very odd given the patch to actually uses the resource in > WebKit hasn't even been upstreamed yet... > > > On Thu, Sep 22, 2011 at 7:55 AM, Tommi <tommi@chromium.org> wrote: > >> FYI - the reason for the revert is that your change broke >> the ClickingMovesFocus unit test on Linux Tests (Views Dbg) 1. >> Please take a look before committing again. >> >> >> On Thu, Sep 22, 2011 at 1:40 PM, <tommi@chromium.org> wrote: >> >>> Reviewers: Fady Samuel, >>> >>> Description: >>> Revert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds. >>> >>> BUG=none >>> TEST=manually >>> >>> >>> Review URL: http://codereview.chromium.**org/7983043<http://codereview.chromium.org/7983043> >>> >>> TBR=fsamuel@chromium.org >>> >>> Please review this at http://codereview.chromium.**org/7989002/<http://codereview.chromium.org/7989... >>> >>> SVN Base: svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/> >>> >>> Affected files: >>> M webkit/glue/webkit_resources.**grd >>> M webkit/glue/**webkitplatformsupport_impl.cc >>> >>> >>> Index: webkit/glue/webkit_resources.**grd >>> ==============================**==============================**======= >>> --- webkit/glue/webkit_resources.**grd (revision 102265) >>> +++ webkit/glue/webkit_resources.**grd (working copy) >>> @@ -52,7 +52,7 @@ >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB" >>> file="resources\mediaplayer_**volume_slider_thumb.png" type="BINDATA" /> >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_HOVER" >>> file="resources\mediaplayer_**volume_slider_thumb_hover.png" >>> type="BINDATA" /> >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_DOWN" >>> file="resources\mediaplayer_**volume_slider_thumb_down.png" >>> type="BINDATA" /> >>> - <if expr="pp_ifdef('touchui') or is_macosx"> >>> + <if expr="is_macosx"> >>> <include name="IDR_OVERHANG_PATTERN" file="resources\overhang_**pattern.png" >>> type="BINDATA" /> >>> </if> >>> <include name="IDC_PAN_EAST" file="resources\pan_east.cur" >>> type="CURSOR" /> >>> Index: webkit/glue/**webkitplatformsupport_impl.cc >>> ==============================**==============================**======= >>> --- webkit/glue/**webkitplatformsupport_impl.cc (revision 102265) >>> +++ webkit/glue/**webkitplatformsupport_impl.cc (working copy) >>> @@ -394,7 +394,7 @@ >>> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_HOVER }, >>> { "**mediaplayerVolumeSliderThumbDo**wn", >>> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_DOWN }, >>> -#if defined(OS_MACOSX) || defined(TOUCH_UI) >>> +#if defined(OS_MACOSX) >>> { "overhangPattern", IDR_OVERHANG_PATTERN }, >>> #endif >>> { "panIcon", IDR_PAN_SCROLL_ICON }, >>> >>> >>> >> >
tommi, I think the this isn't the cause of the failure. The test seems to be still failing after revert. I actually looked into this last night, but reverting to somewhere around Sept 20ish didn't fix the issue. May be we need help from author of this test? - oshima On Thu, Sep 22, 2011 at 6:48 AM, Fady Samuel <fsamuel@chromium.org> wrote: > This seems very, very odd given the patch to actually uses the resource in > WebKit hasn't even been upstreamed yet... > > On Thu, Sep 22, 2011 at 7:55 AM, Tommi <tommi@chromium.org> wrote: > >> FYI - the reason for the revert is that your change broke >> the ClickingMovesFocus unit test on Linux Tests (Views Dbg) 1. >> Please take a look before committing again. >> >> >> On Thu, Sep 22, 2011 at 1:40 PM, <tommi@chromium.org> wrote: >> >>> Reviewers: Fady Samuel, >>> >>> Description: >>> Revert 102217 - Enable the Overhang pattern resource on TOUCH_UI builds. >>> >>> BUG=none >>> TEST=manually >>> >>> >>> Review URL: http://codereview.chromium.**org/7983043<http://codereview.chromium.org/7983043> >>> >>> TBR=fsamuel@chromium.org >>> >>> Please review this at http://codereview.chromium.**org/7989002/<http://codereview.chromium.org/7989... >>> >>> SVN Base: svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/> >>> >>> Affected files: >>> M webkit/glue/webkit_resources.**grd >>> M webkit/glue/**webkitplatformsupport_impl.cc >>> >>> >>> Index: webkit/glue/webkit_resources.**grd >>> ==============================**==============================**======= >>> --- webkit/glue/webkit_resources.**grd (revision 102265) >>> +++ webkit/glue/webkit_resources.**grd (working copy) >>> @@ -52,7 +52,7 @@ >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB" >>> file="resources\mediaplayer_**volume_slider_thumb.png" type="BINDATA" /> >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_HOVER" >>> file="resources\mediaplayer_**volume_slider_thumb_hover.png" >>> type="BINDATA" /> >>> <include name="IDR_MEDIAPLAYER_VOLUME_**SLIDER_THUMB_DOWN" >>> file="resources\mediaplayer_**volume_slider_thumb_down.png" >>> type="BINDATA" /> >>> - <if expr="pp_ifdef('touchui') or is_macosx"> >>> + <if expr="is_macosx"> >>> <include name="IDR_OVERHANG_PATTERN" file="resources\overhang_**pattern.png" >>> type="BINDATA" /> >>> </if> >>> <include name="IDC_PAN_EAST" file="resources\pan_east.cur" >>> type="CURSOR" /> >>> Index: webkit/glue/**webkitplatformsupport_impl.cc >>> ==============================**==============================**======= >>> --- webkit/glue/**webkitplatformsupport_impl.cc (revision 102265) >>> +++ webkit/glue/**webkitplatformsupport_impl.cc (working copy) >>> @@ -394,7 +394,7 @@ >>> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_HOVER }, >>> { "**mediaplayerVolumeSliderThumbDo**wn", >>> IDR_MEDIAPLAYER_VOLUME_SLIDER_**THUMB_DOWN }, >>> -#if defined(OS_MACOSX) || defined(TOUCH_UI) >>> +#if defined(OS_MACOSX) >>> { "overhangPattern", IDR_OVERHANG_PATTERN }, >>> #endif >>> { "panIcon", IDR_PAN_SCROLL_ICON }, >>> >>> >>> >> > |
|||||||||||||||||||||||||||||||||||||||||||||||
