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

Issue 668485 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Do not force offline version when opening snippets with an offline page.

Project Member Reported by vitaliii@chromium.org, Nov 24 2016

Issue description

Version: Canary 57.0.2929.0
OS: Android

What steps will reproduce the problem?
(1) Open Chrome;
(2) Offline the first article suggestion;
(3) After it is downloaded, tap on it;

What is the expected result?
Simply opening its URL if the user is online;

What happens instead?
Offline version of the page is opened;

As it was discussed by email, we want such behavior and this agrees with Most Likely tiles.

This will need to be merged into M-56.
 

Comment 1 by fi...@chromium.org, Nov 24 2016

Labels: -Type-Bug-Regression -M-57 zine-triaged Type-Bug
Correct! The behavior depends on the section:
- Clicks in the "Downloads section" should always open the offline version of the page
- Clicks in all other sections should simply follow the target URL (=open the online version). If the user is on a bad connection or even offline, it's up to the Offline feature to re-route the user to the offline version of the page.

Comment 2 by fi...@chromium.org, Nov 24 2016

Forgot to mention: Pls fix and merge back to M-56. Thx!

Comment 3 by fi...@chromium.org, Nov 24 2016

Labels: zine-downloads-v1
Labels: zine-client-v1
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 25 2016

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

commit 6f538029f3659bf667470deca0b22abdcd7cafbd
Author: vitaliii <vitaliii@chromium.org>
Date: Fri Nov 25 07:22:51 2016

[NTP] Do not force offline version when opening non-download snippets.

Previously we always opened an offline page, if there was one. This CL
simply opens the URL for non-download snippets and then Offline Pages
decide whether to open its offline page. For Downloads nothing changed.

BUG= 668485 

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

[modify] https://crrev.com/6f538029f3659bf667470deca0b22abdcd7cafbd/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Status: Fixed (was: Started)

Comment 8 by treib@chromium.org, Nov 28 2016

Labels: Merge-Request-56

Comment 9 by dimu@chromium.org, Nov 28 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 10 by sheriffbot@chromium.org, Dec 1 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 1 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9a10dcad458b7a05ca295a25ae621c30f264f05

commit c9a10dcad458b7a05ca295a25ae621c30f264f05
Author: Marc Treib <treib@chromium.org>
Date: Thu Dec 01 14:25:02 2016

[NTP] Do not force offline version when opening non-download snippets.

Previously we always opened an offline page, if there was one. This CL
simply opens the URL for non-download snippets and then Offline Pages
decide whether to open its offline page. For Downloads nothing changed.

BUG= 668485 

Review-Url: https://codereview.chromium.org/2525323003
Cr-Commit-Position: refs/heads/master@{#434445}
(cherry picked from commit 6f538029f3659bf667470deca0b22abdcd7cafbd)

Review URL: https://codereview.chromium.org/2546583004 .

Cr-Commit-Position: refs/branch-heads/2924@{#245}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/c9a10dcad458b7a05ca295a25ae621c30f264f05/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 2 2016

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

commit c00bcb8e7423116cd8c10a56d26bc7cdc5005e8b
Author: avayvod <avayvod@chromium.org>
Date: Fri Dec 02 07:29:15 2016

Revert of [NTP] Do not force offline version when opening non-download snippets. (patchset #1 id:1 of https://codereview.chromium.org/2546583004/ )

Reason for revert:
Broke the official 56 build: https://bugs.chromium.org/p/chromium/issues/detail?id=670559

Seems like https://codereview.chromium.org/2514343003 has to be merged before (doesn't seem to be in the 56 branch according to https://chromium.googlesource.com/chromium/src/+/56.0.2924.14/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java)

Original issue's description:
> [NTP] Do not force offline version when opening non-download snippets.
>
> Previously we always opened an offline page, if there was one. This CL
> simply opens the URL for non-download snippets and then Offline Pages
> decide whether to open its offline page. For Downloads nothing changed.
>
> BUG= 668485 
>
> Review-Url: https://codereview.chromium.org/2525323003
> Cr-Commit-Position: refs/heads/master@{#434445}
> (cherry picked from commit 6f538029f3659bf667470deca0b22abdcd7cafbd)
>
> Committed: https://chromium.googlesource.com/chromium/src/+/c9a10dcad458b7a05ca295a25ae621c30f264f05

TBR=treib@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 668485 

Review-Url: https://codereview.chromium.org/2541123005
Cr-Commit-Position: refs/branch-heads/2924@{#285}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/c00bcb8e7423116cd8c10a56d26bc7cdc5005e8b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Cc: treib@chromium.org
Labels: -merge-merged-2924 Merge-Approved-56
Resetting labels since the merge was reverted. I'll land a fixed merge CL today.
Project Member

Comment 14 by sheriffbot@chromium.org, Dec 5 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 5 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d36250aae160e1a5f37b80638bd5ef588bde6e1

commit 5d36250aae160e1a5f37b80638bd5ef588bde6e1
Author: Marc Treib <treib@chromium.org>
Date: Mon Dec 05 15:00:18 2016

[NTP] Do not force offline version when opening non-download snippets.

Previously we always opened an offline page, if there was one. This CL
simply opens the URL for non-download snippets and then Offline Pages
decide whether to open its offline page. For Downloads nothing changed.

BUG= 668485 

Review-Url: https://codereview.chromium.org/2525323003
Cr-Commit-Position: refs/heads/master@{#434445}
(cherry picked from commit 6f538029f3659bf667470deca0b22abdcd7cafbd)

Review URL: https://codereview.chromium.org/2553663002 .

Cr-Commit-Position: refs/branch-heads/2924@{#328}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/5d36250aae160e1a5f37b80638bd5ef588bde6e1/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Sign in to add a comment