Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2241393003:
Offline Pages: Add custom tabs namespace to tab helper. (Closed)
|
Created:
4 years, 4 months ago by dewittj Modified:
4 years, 4 months ago Reviewers:
jianli CC:
chromium-reviews, romax+watch_chromium.org, fgorski+watch_chromium.org, dewittj+watch_chromium.org, petewil+watch_chromium.org, chili+watch_chromium.org, dimich+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionOffline Pages: Add custom tabs namespace to tab helper.
This regressed the custom tabs integration.
BUG=638259
Committed: https://crrev.com/4a8d277960be5fa6a91d3e5efa9f5378cf7086a4
Cr-Commit-Position: refs/heads/master@{#412113}
Patch Set 1 #Patch Set 2 : Generalize to allow all namespaces. #Messages
Total messages: 18 (8 generated)
Description was changed from ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= ========== to ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= ==========
dewittj@chromium.org changed reviewers: + jianli@chromium.org
lgtm It would be good if we can also extend the check to other namespaces, like download. Or maybe we should simply drop namespace checks/
The CQ bit was checked by dewittj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I removed the namespace check except for "Last 1". PTAL.
still lgtm
The CQ bit was checked by dewittj@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= ========== to ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= ========== to ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= Committed: https://crrev.com/4a8d277960be5fa6a91d3e5efa9f5378cf7086a4 Cr-Commit-Position: refs/heads/master@{#412113} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/4a8d277960be5fa6a91d3e5efa9f5378cf7086a4 Cr-Commit-Position: refs/heads/master@{#412113}
Message was sent while issue was closed.
I do't understand the description nor the reason of this CL from looking at it. Why there is no bug number? The description does not match what is done. What exactly regressed? Please reply with details and please let keep our CLs more self-explanatory.
Message was sent while issue was closed.
Description was changed from ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG= Committed: https://crrev.com/4a8d277960be5fa6a91d3e5efa9f5378cf7086a4 Cr-Commit-Position: refs/heads/master@{#412113} ========== to ========== Offline Pages: Add custom tabs namespace to tab helper. This regressed the custom tabs integration. BUG=638259 Committed: https://crrev.com/4a8d277960be5fa6a91d3e5efa9f5378cf7086a4 Cr-Commit-Position: refs/heads/master@{#412113} ==========
Message was sent while issue was closed.
On 2016/08/16 02:13:21, Dmitry Titov wrote: > I do't understand the description nor the reason of this CL from looking at it. > Why there is no bug number? The description does not match what is done. > What exactly regressed? Please reply with details and please let keep our CLs > more self-explanatory. The description got out of date. I added a bug number post-hoc, but there was no bug. Basically the tab helper was only showing pages from 3 namespaces, but we've been adding several more. This list got out of date and did not include CCT as well as a few others. Those pages would only be available by navigating to the offline file:// URL.
Message was sent while issue was closed.
Got it, thanks! On Tue, Aug 16, 2016 at 9:14 AM <dewittj@chromium.org> wrote: > On 2016/08/16 02:13:21, Dmitry Titov wrote: > > I do't understand the description nor the reason of this CL from looking > at > it. > > Why there is no bug number? The description does not match what is done. > > What exactly regressed? Please reply with details and please let keep > our CLs > > more self-explanatory. > > The description got out of date. I added a bug number post-hoc, but there > was > no bug. > > Basically the tab helper was only showing pages from 3 namespaces, but > we've > been adding several more. This list got out of date and did not include > CCT as > well as a few others. Those pages would only be available by navigating to > the > offline file:// URL. > > https://codereview.chromium.org/2241393003/ > -- 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. |
