Enable chrome custom tabs in Nightly

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: snorp, Assigned: sebastian)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

Could be a little annoying for Nightly folks since google search bar, twitter, etc, could use it. The defaults get cleared every time you update Nightly, though, so it probably won't actually get used unless you pretty aggressively re-make it the default.
Created attachment 8762775 [details] [diff] [review]
Enable Chrome Custom Tabs in Nightly be default
Attachment #8762775 - Flags: review?(s.kaspari)
(Assignee)

Comment 2

2 years ago
Comment on attachment 8762775 [details] [diff] [review]
Enable Chrome Custom Tabs in Nightly be default

Review of attachment 8762775 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure if it's a good idea to enable this at this stage (but I'll r+ and you decide). I used a custom build for some time and I was annoyed by the "white screen" problem pretty fast. It's just happening too often now that a lot of apps use custom tabs. However it will only affect users that use Nightly as their default browser (That's me!).

I was thinking about maybe adding a (user-visible) preference to disable custom tabs: We'd still pretend to support them but instead of opening the custom tab we just open a normal tab in the browser (or queue the tab). This would be helpful here, but it also allows the user to regain control and decide how links should be treated by the browser.
Attachment #8762775 - Flags: review?(s.kaspari) → review+
(Assignee)

Updated

2 years ago
Assignee: nobody → snorp
(Assignee)

Updated

2 years ago
Blocks: 1285926
(Assignee)

Updated

2 years ago
Blocks: 1208655
(Assignee)

Comment 3

2 years ago
Comment on attachment 8762775 [details] [diff] [review]
Enable Chrome Custom Tabs in Nightly be default

I want to land this witch the patch in bug 1285926 - which will introduce a preference (default: off). This way we can have test this in Nightly but it won't affect Nightly users that do not enable it.

@glandium: Can you have a quick look at the patch? I did review it in the past and it seemed to work locally, but I want to make sure that it's following our best practices.
Attachment #8762775 - Flags: review?(mh+mozilla)
(Assignee)

Updated

2 years ago
Assignee: snorp → s.kaspari
Status: NEW → ASSIGNED
Comment on attachment 8762775 [details] [diff] [review]
Enable Chrome Custom Tabs in Nightly be default

Review of attachment 8762775 [details] [diff] [review]:
-----------------------------------------------------------------

This is how you'd do what you want within the current framework, but this sucks, so I'm going to file a bug so that you can just do the right thing in a simpler manner:

replace default=false with default=delayed_getattr(milestone, 'is_nightly')

::: mobile/android/moz.configure
@@ +36,5 @@
>  project_flag('MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE',
>               help='Background service for downloading additional content at runtime',
>               default=True)
>  
> +@depends(milestone)

@depends(milestone, '--help')

@@ -40,3 @@
>  project_flag('MOZ_ANDROID_CUSTOM_TABS',
> -             help='Enable support for Android custom tabs',
> -             default=False)

default=are_custom_tabs_enabled)

@@ +44,4 @@
>  project_flag('MOZ_ANDROID_CUSTOM_TABS',
> +             help='Enable support for Android custom tabs')
> +
> +set_config('MOZ_ANDROID_CUSTOM_TABS', are_custom_tabs_enabled)

And please remove this line.
Attachment #8762775 - Flags: review?(mh+mozilla) → review-
Depends on: 1287023
(Assignee)

Comment 5

2 years ago
Created attachment 8771494 [details]
Bug 1280148 - Enable Chrome Custom Tabs in Nightly be default

Review commit: https://reviewboard.mozilla.org/r/64618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64618/
Attachment #8771494 - Flags: review?(mh+mozilla)
(Assignee)

Updated

2 years ago
Attachment #8762775 - Attachment is obsolete: true
(Assignee)

Comment 6

2 years ago
This is the updated patch. If bug 1287023 will land before this one then feel free to clear the review and I'll update the patch to use the new syntax.
Comment on attachment 8771494 [details]
Bug 1280148 - Enable Chrome Custom Tabs in Nightly be default

https://reviewboard.mozilla.org/r/64618/#review61730

::: mobile/android/moz.configure:46
(Diff revision 1)
> +def are_custom_tabs_enabled(milestone):
> +    return milestone.is_nightly
> +
>  project_flag('MOZ_ANDROID_CUSTOM_TABS',
>               help='Enable support for Android custom tabs',
> -             default=False)
> +             default=are_custom_tabs_enabled)

Bug 1287023 landed on inbound.
Attachment #8771494 - Flags: review?(mh+mozilla)
(Assignee)

Comment 8

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/d8a5ea157504793ff2bb2b5161103154a31f08e3
Bug 1280148 - Enable Chrome Custom Tabs in Nightly be default r=glandium
(Assignee)

Comment 9

2 years ago
Argh. I pushed the patch to fx-team instead of reviewboard. Glandium, can you give it a review after it has landed? It's following your advice from the previous reviews so I won't backout for now.
Flags: needinfo?(mh+mozilla)
r+
Flags: needinfo?(mh+mozilla)

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d8a5ea157504
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.