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

Issue 687972 link

Starred by 3 users
Status: Fixed
Owner:
treib@chromium.org
Closed: Feb 2017
Cc:
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug

Blocking:
issue 583289



Sign in to add a comment

[Local NTP] Cleanup

Project Member Reported by treib@chromium.org, Feb 2 2017 
The local NTP could use some cleanup before we make it the main one. In particular:

- Support for the "icons" design should be removed. It was never fully implemented and has long been abandoned.

- Much of the html is actually created dynamically in the js, for no particular reason that I can see.

- The counting of loaded elements in the single iframe is super-convoluted and (we suspect) broken in some cases. This needs an overhaul/clean reimplementation. [This affects both the local and the remote NTP.]
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 8 2017 

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

commit 5af7397e1d8ae4d18d03bbfc174227fc57d6a15c
Author: treib <treib@chromium.org>
Date: Wed Feb 08 15:38:52 2017

[Local NTP] Cleanup: Don't create HTML elements dynamically

Before this CL, many of the HTML elements on the local NTP were created
dynamically in JS, for no good reason. This CL moves the element
definitions into HTML where they belong.

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

Review-Url: https://codereview.chromium.org/2684593002
Cr-Commit-Position: refs/heads/master@{#449001}

[modify] https://crrev.com/5af7397e1d8ae4d18d03bbfc174227fc57d6a15c/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/5af7397e1d8ae4d18d03bbfc174227fc57d6a15c/chrome/browser/resources/local_ntp/local_ntp.html
[modify] https://crrev.com/5af7397e1d8ae4d18d03bbfc174227fc57d6a15c/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/5af7397e1d8ae4d18d03bbfc174227fc57d6a15c/chrome/test/data/local_ntp_browsertest.html
[modify] https://crrev.com/5af7397e1d8ae4d18d03bbfc174227fc57d6a15c/chrome/test/data/local_ntp_browsertest.js
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 14 2017 

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

commit cfa2dfc45824fd0b7c912ee050e441807b1d3d09
Author: treib <treib@chromium.org>
Date: Tue Feb 14 14:10:30 2017

Cleanup in NTP single-iframe

- Add a few comments
- Rename some functions for (IMO) better clarity
- Remove "file://" as an allowed scheme: support for it has been removed
  a long time ago; see https://codereview.chromium.org/1924773002

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

Review-Url: https://codereview.chromium.org/2693143002
Cr-Commit-Position: refs/heads/master@{#450346}

[modify] https://crrev.com/cfa2dfc45824fd0b7c912ee050e441807b1d3d09/chrome/browser/resources/local_ntp/most_visited_single.js

Comment 6 by treib@chromium.org, Feb 21 2017

Status: Fixed (was: Started)
I don't have any more immediate cleanups on my list, so let's consider this done.
►

Sign in to add a comment