1269051 - Disable screenshot in bookmarks via a switchboard flag rather than at compile-time

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Follow-up to compile-time disable, bug 1266820.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Margaret, it's non-trivial to do this because we need to know if we're in the experiment in LocalBrowserDB.getBookmarksInFolder (to create the smart folder) which does not have a reference to the Context. How important is it that we do this? fwiw, this could be revisited when we're implementing this feature if we'd like to use actual bucketing.

If we were to implement this, some solutions off the top of my head:
* Pass a context into LocalBrowserDB and set a member var reporting the state of the experiment – LocalBrowserDB is used outside of BrowserApp so this isn't entirely appropriate and can have weird side effects by accessing shared prefs values. Also, many callsites to update.
* Add a static reference in BrowserApp that is updated in onCreate (or similar) and that LocalBrowserDB can access – icky global state, but probably the better solution (it's like a runtime constant).

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1269051 - (WIP) Use switchboard to disable screenshots in bookmarks. r=margaret

Review commit: https://reviewboard.mozilla.org/r/49877/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49877/

Comment 3

[:Margaret Leibovic]

If it's difficult (and it sounds like it is), we don't need to worry about moving this behind a switchboard flag right now.

However, in the future, I imagine we may want to control the UI of this feature with a switchboard flag, but we can cross that bridge when we get to it.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Unassigning until that time.

Comment 5

[Nevin Chen(Not active on Bugzilla)]

I'll take this. Is this bug still valid?

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I don't think this is still valid. 

Correct me if I'm wrong, but I think this is left over from when we were experimenting with a "Screenshots" smart folder in Bookmarks.

@mcomella?

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

> I think this is left over from when we were experimenting with a "Screenshots" smart folder in Bookmarks.

Yep - I agree this is no longer valid (unless this is a feature that we intend to ship).

Comment 8

[Nevin Chen(Not active on Bugzilla)]

I'll close this bug per comment 7