Reduce usage of webkit CSS prefixes in chrome/browser/resources

Reduce usage of webkit CSS prefixes in chrome/browser/resources

Animations, transitions, transforms and perspective don't require
webkit prefix.

BUG=668650
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

[Eric Willigers, 2016-11-25 19:04:10]

Description was changed from

==========
Reduce usage of webkit prefixes in chrome/browser/resources

Animations, transitions, transforms and perspective don't require
webkit prefix.

BUG=668650
==========

to

==========
Reduce usage of webkit prefixes in chrome/browser/resources

Animations, transitions, transforms and perspective don't require
webkit prefix.

BUG=668650
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Eric Willigers, 2016-11-25 19:11:36]

The CQ bit was checked by ericwilligers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 19:11:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eric Willigers, 2016-11-25 19:11:57]

ericwilligers@chromium.org changed reviewers:
+ dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-25 20:25:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 20:25:40]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-11-29 06:25:04]

please run css_checker_test.py, and possibly revert changes to it.  it's testing
that -webkit- prefixed things are sorted correctly.  you can substitute for
other -webkit- prefixed things that are likely to say prefixed if you'd like.

also, re-apply indent and alpha and CSSOM nits from other CL

[Eric Willigers, 2016-11-30 02:02:08]

Patchset #5 (id:80001) has been deleted

[Eric Willigers, 2016-11-30 02:06:34]

On 2016/11/29 06:25:04, Dan Beam wrote:
> please run css_checker_test.py, and possibly revert changes to it.  it's
testing
> that -webkit- prefixed things are sorted correctly.  you can substitute for
> other -webkit- prefixed things that are likely to say prefixed if you'd like.

Reverted.

> also, re-apply indent and alpha and CSSOM nits from other CL

indent - fixed.

alpha - done, for CSS files this is already enforced by pre-submit trigger.
Skipped chromeos/chromevox/chromevox/injected/ui/search_widget.js and
net_internals/hsts_view.js 


CSSOM - done, by updating all chrome/browser/ files other than
chrome/browser/resources/md* 
I can split this CL into perhaps 4 CLs if that makes review easier.

[Eric Willigers, 2016-11-30 02:06:52]

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js
(left):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js:268:
containerNode.style['opacity'] = '0.0';
I have left these unsorted.

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth/offline.css (right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth/offline.css:151: transition: all 218ms;
"-webkit-transition: all 218ms;" was redundant.

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth/offline.css:175: transition: all 0ms;
"-webkit-transition: all 0ms;" was redundant.

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/net_internals/hsts_view.js (left):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/net_internals/hsts_view.js:185:
element.style.backgroundColor = '#fffccf';
I have left these unsorted.

[Eric Willigers, 2016-11-30 11:19:31]

Description was changed from

==========
Reduce usage of webkit prefixes in chrome/browser/resources

Animations, transitions, transforms and perspective don't require
webkit prefix.

BUG=668650
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Reduce usage of webkit CSS prefixes in chrome/browser/resources

Animations, transitions, transforms and perspective don't require
webkit prefix.

BUG=668650
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-12-02 01:54:34]

well that was fun

lgtm

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth/offline.css (right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth/offline.css:175: transition: all 0ms;
On 2016/11/30 02:06:52, Eric Willigers wrote:
> "-webkit-transition: all 0ms;" was redundant.

arguably this should just be 0

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/print_preview/previewarea/margin_control.js
(right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/previewarea/margin_control.js:385: *
Called when opacity CSS transition ends.
nit: @param {Event} event

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/print_preview/print_preview_animations.js (right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/print_preview_animations.js:86: * @param
{transitionEvent} event The event that triggered this listener.
just Event

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/print_preview_animations.js:98: * @param
{animationEvent} event The event that triggered this listener.
and here

[Eric Willigers, 2016-12-02 20:11:55]

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth/offline.css (right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth/offline.css:175: transition: all 0ms;
On 2016/12/02 01:54:34, Dan Beam wrote:
> On 2016/11/30 02:06:52, Eric Willigers wrote:
> > "-webkit-transition: all 0ms;" was redundant.
> 
> arguably this should just be 0

0 without units is only for lengths.
https://developer.mozilla.org/en/docs/Web/CSS/time

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/print_preview/previewarea/margin_control.js
(right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/previewarea/margin_control.js:385: *
Called when opacity CSS transition ends.
On 2016/12/02 01:54:34, Dan Beam wrote:
> nit: @param {Event} event

Done.

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/print_preview/print_preview_animations.js (right):

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/print_preview_animations.js:86: * @param
{transitionEvent} event The event that triggered this listener.
On 2016/12/02 01:54:34, Dan Beam wrote:
> just Event

Done.

https://codereview.chromium.org/2535573002/diff/60001/chrome/browser/resource...
chrome/browser/resources/print_preview/print_preview_animations.js:98: * @param
{animationEvent} event The event that triggered this listener.
On 2016/12/02 01:54:34, Dan Beam wrote:
> and here

Done.

[Eric Willigers, 2016-12-02 20:12:04]

The CQ bit was checked by ericwilligers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 20:12:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-02 21:17:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 21:17:49]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dan Beam, 2017-02-07 21:59:50]

so, what's up with this CL?

[Eric Willigers, 2017-02-09 04:39:45]

The CQ bit was checked by ericwilligers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-09 04:40:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 06:07:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-09 06:07:11]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Eric Willigers, 2017-02-09 07:30:20]

On 2017/02/07 21:59:50, Dan Beam wrote:
> so, what's up with this CL?

I never tracked down what caused the test failures. I'll investigate tomorrow.

[Eric Willigers, 2017-02-10 06:44:53]

The CQ bit was checked by ericwilligers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-10 06:45:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 06:48:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-10 06:48:15]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Eric Willigers, 2017-02-10 10:01:03]

The CQ bit was checked by ericwilligers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-10 10:01:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 11:43:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-10 11:43:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Eric Willigers, 2017-02-10 19:00:14]

On 2017/02/09 07:30:20, Eric Willigers wrote:
> On 2017/02/07 21:59:50, Dan Beam wrote:
> > so, what's up with this CL?
> 
> I never tracked down what caused the test failures. I'll investigate tomorrow.

Everything is passing except FirstRunUIBrowserTest.FirstRunFlow on ChromeOS.
Same for https://codereview.chromium.org/2535493002/

Should I combine the two CLs, or is there an obvious file or two that I should
move from one CL to the other?

Should I be checking out and building ChromeOS? I usually work on Blink, so all
my past changes developed on Linux have "just worked" on ChromeOS, Mac, Windows,
Android.

[Dan Beam, 2017-02-13 06:24:39]

https://codereview.chromium.org/2535573002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/first_run/first_run.js (right):

https://codereview.chromium.org/2535573002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/chromeos/first_run/first_run.js:24: element, visible,
opt_transitionDuration, opt_onFinished) {
i'm fairly sure that something in this method is blowing up the firstrun tests

[chromium-reviews, 2017-02-13 06:47:09]

Note that if I combine the ui/ and chrome/ into a large changeset, it
passes:
https://codereview.chromium.org/2691883002/

But that is far too large.

There is no need to be changing transitions, animations and
transform/perspective all at the same time.

I have split out the animations and transform/perspective changes into
their own patches, and the tests all pass:
https://codereview.chromium.org/2693793002/
https://codereview.chromium.org/2688353003/
https://codereview.chromium.org/2692803003/
https://codereview.chromium.org/2689163002/

If you are happy with these small patches, I can land these 4.

One option would be to then combine the ui/ and chrome/  transitions
changes into a single 5th patch.







On Mon, Feb 13, 2017 at 5:24 PM, <dbeam@chromium.org> wrote:

>
> https://codereview.chromium.org/2535573002/diff/200001/
> chrome/browser/resources/chromeos/first_run/first_run.js
> File chrome/browser/resources/chromeos/first_run/first_run.js (right):
>
> https://codereview.chromium.org/2535573002/diff/200001/
> chrome/browser/resources/chromeos/first_run/first_run.js#newcode24
> chrome/browser/resources/chromeos/first_run/first_run.js:24: element,
> visible, opt_transitionDuration, opt_onFinished) {
> i'm fairly sure that something in this method is blowing up the firstrun
> tests
>
> https://codereview.chromium.org/2535573002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Eric Willigers, 2017-02-13 07:15:54]

On 2017/02/13 06:24:39, Dan Beam wrote:
>
https://codereview.chromium.org/2535573002/diff/200001/chrome/browser/resourc...
> File chrome/browser/resources/chromeos/first_run/first_run.js (right):
> 
>
https://codereview.chromium.org/2535573002/diff/200001/chrome/browser/resourc...
> chrome/browser/resources/chromeos/first_run/first_run.js:24: element, visible,
> opt_transitionDuration, opt_onFinished) {
> i'm fairly sure that something in this method is blowing up the firstrun tests

That code calls ensureTransitionEndEvent. We are changing this to ensure
transition end events instead of ensuring webkit transition end events.

As ensureTransitionEndEvent is defined in ui/webui/resources/js/util.js but
called in chrome/browser, we need to change both at the same time.

[Eric Willigers, 2017-02-15 06:46:04]

The transforms and animations changes have landed.

The transitions changes have been included in
https://codereview.chromium.org/2691883002/
(unchanged except for chrome/browser/resources/history/history.css - reorder
flex)

[Eric Willigers, 2017-02-20 02:17:11]

Landed as
https://codereview.chromium.org/2688353003/
https://codereview.chromium.org/2689163002/
https://codereview.chromium.org/2691883002/