New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 761942 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Feature

Blocked on:
issue 761824

Blocking:
issue 583290
issue 761994



Sign in to add a comment

Show non-interactive doodles on Desktop local NTP

Project Member Reported by sfiera@chromium.org, Sep 5 2017

Issue description

LogoService already supports enough to get non-interactive (SIMPLE, ANIMATED) doodles to work on the Local NTP. Start with that, and move on to interactives second.
 
Labels: zine-triaged Type-Feature
Blocking: 583290
Blocking: 761994

Comment 4 by sfiera@chromium.org, Sep 11 2017

Labels: -M-62 M-63
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/569806f1a25911040361ed880d1d27791f9b5c41

commit 569806f1a25911040361ed880d1d27791f9b5c41
Author: Chris Pickel <sfiera@chromium.org>
Date: Wed Sep 20 07:38:47 2017

Local NTP: support simple doodles

Checked against the ddljson test doodles, plus today's doodle in Mexico.
A lot more testing is needed, for doodles changing at various times, but
that's something that I think automated tests will be better for, since
timing is tricky.

Animated doodles still to come.

The Google logo fades out before a doodle fades in. Maybe it would be
preferable to start with nothing, but that complicates support for NTPs
with the feature disabled. This will come up in implementation review.

Bug:  761942 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9469cb6cdbcee3b60680dade357faee301175257
Reviewed-on: https://chromium-review.googlesource.com/671012
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503077}
[modify] https://crrev.com/569806f1a25911040361ed880d1d27791f9b5c41/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/569806f1a25911040361ed880d1d27791f9b5c41/chrome/browser/resources/local_ntp/local_ntp.html
[modify] https://crrev.com/569806f1a25911040361ed880d1d27791f9b5c41/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/569806f1a25911040361ed880d1d27791f9b5c41/chrome/browser/search/local_ntp_source.cc
[modify] https://crrev.com/569806f1a25911040361ed880d1d27791f9b5c41/chrome/test/data/local_ntp/local_ntp_browsertest.html

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21315c9b3de7d37c058eedd836e2ac6deb1aa2fd

commit 21315c9b3de7d37c058eedd836e2ac6deb1aa2fd
Author: Chris Pickel <sfiera@chromium.org>
Date: Thu Sep 21 08:45:09 2017

Local NTP: support animated doodles

Missing: image preloading. The remote NTP preloaded animatedUrl
unconditionally at page load, but allowed the animated image to be
swapped in even before loading finished, allowing for the possibility of
a flash.

Bug:  761942 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib9c0319e28eb41e8d8fceb68f86ad5ebce8d54fc
Reviewed-on: https://chromium-review.googlesource.com/674690
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503392}
[modify] https://crrev.com/21315c9b3de7d37c058eedd836e2ac6deb1aa2fd/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/21315c9b3de7d37c058eedd836e2ac6deb1aa2fd/chrome/browser/resources/local_ntp/local_ntp.js

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57ba63e42e30770808828c82a5689cc21e97149b

commit 57ba63e42e30770808828c82a5689cc21e97149b
Author: Chris Pickel <sfiera@chromium.org>
Date: Thu Sep 21 09:22:03 2017

[Local NTP] Position doodles by top edge

This accommodates doodles of differing heights, where some (like today's
Germany doodle) should extend all the way to the fakebox, but others
shouldn't.

The positions have been obtained by the scientific method of: 1) making
sure the default Google logo doesn't move and 2) adjusting doodles until
they look like today's homepage doodle.

Bug:  761942 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I588c536f60447b72a256e07cbb758e9bd4200624
Reviewed-on: https://chromium-review.googlesource.com/675404
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503396}
[modify] https://crrev.com/57ba63e42e30770808828c82a5689cc21e97149b/chrome/browser/resources/local_ntp/local_ntp.css

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/617a67e3f969086f0752466d98d5bef25ee15e39

commit 617a67e3f969086f0752466d98d5bef25ee15e39
Author: Marc Treib <treib@chromium.org>
Date: Mon Sep 25 16:48:51 2017

Local NTP: show cached doodles immediately (no fade)

This hides both logo and doodle until the cached doodle result comes in.
As a side effect, this simplifies the JS logic a bit, since there's no
possibility of interrupting a pending transition anymore.

Tested:
- Feature disabled
- No cached doodle but a fresh one (logo fades out, doodle fades in)
- Cached doodle but no fresh one (doodle fades out, logo fades in).
- Cached doodle but a different fresh one (old one fades out, new one in).

Bug:  761942 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If09a632548d6df05c780709b5359c238e177cfa7
Reviewed-on: https://chromium-review.googlesource.com/678512
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504079}
[modify] https://crrev.com/617a67e3f969086f0752466d98d5bef25ee15e39/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/617a67e3f969086f0752466d98d5bef25ee15e39/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/617a67e3f969086f0752466d98d5bef25ee15e39/chrome/browser/search/local_ntp_source.cc

Comment 9 by sfiera@chromium.org, Sep 28 2017

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd

commit 881d4ac518be868bb6ea9b5294e8dfa4b0a212cd
Author: Chris Pickel <sfiera@chromium.org>
Date: Fri Oct 13 10:38:57 2017

Local NTP accessibility: use <button>, not <a>

For animated CTA images, there should be no URL shown in the lower left
on mouseover (it's JS), but it should be possible to activate the link
with tab/enter. <button> tags have these properties, but <a> can only
have one or the other.

Bug:  761942 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I299f187330ab58507cdcdb3722f840b9edfaa17e
Reviewed-on: https://chromium-review.googlesource.com/709275
Commit-Queue: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508666}
[modify] https://crrev.com/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd/chrome/browser/resources/local_ntp/local_ntp.html
[modify] https://crrev.com/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd/chrome/browser/ui/search/local_ntp_browsertest.cc
[modify] https://crrev.com/881d4ac518be868bb6ea9b5294e8dfa4b0a212cd/chrome/test/data/local_ntp/local_ntp_browsertest.html

Sign in to add a comment