Closed Bug 1377294 Opened 7 years ago Closed 7 years ago

(photon) - Let some toolbar views become theme-able

Tracking

(firefox56 fixed)

Status:

RESOLVED FIXED

Milestone:

Firefox 56

Tracking Flags:

Tracking

Status

firefox56

---

fixed

People

(Reporter: walkingice, Assigned: walkingice)

References

Details

Attachments

(4 files)

Bug 1377294 - duplicate xml resource files 7 years ago Julian Chu [:walkingice] 59 bytes, text/x-review-board-request	cnevinchen : review+ jwu : review+	Details
Bug 1377294 - Let some icon in ToolbarEditLayout become Theme-able 7 years ago Julian Chu [:walkingice] 59 bytes, text/x-review-board-request	cnevinchen : review+ jwu : review+	Details
Bug 1377294 - Let Site-Identity becomes theme-able 7 years ago Julian Chu [:walkingice] 59 bytes, text/x-review-board-request	cnevinchen : review+ jwu : review+	Details
Bug 1377294 - ToolbarDisplay layout color span supports Pirvate mode 7 years ago Julian Chu [:walkingice] 59 bytes, text/x-review-board-request	cnevinchen : review+ jwu : review+	Details

Julian Chu [:walkingice]

Assignee

Description

•

7 years ago

There are some views in Toolbar, such as

* Mic icon
* QRCode icon
* Search icon
* Site identity

In Australis-flavor, they always have same appearance regardless of normal/private mode. However in Photon-flavor, their appearance should changed.

This bug tries to changes their class type from `Foo` to `ThemedFoo`.

Comment hidden (mozreview-request)

Similiar to Bug 1375448. To duplicate and indent some resource files
which will be used in upcoming patches, to prevent effect Australis
flavor.

* toolbar_display_layout
* url_bar_title.xml
* url_bar_title_hint.xml
* drawable/url_bar_bg.xml
* browser_toolbar.xml
* toolbar_edit_layout.xml

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

Comment hidden (mozreview-request)

* search icon
* qrcode
* mic

In Australia-flavor, it still use not-theme-able drawable, hence its
appearance won't change even in private mode. In Photon-flavor, these
icons should change its color base on normal/private mode.

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

Comment hidden (mozreview-request)

In Australis flavor, it use same color then user should not aware of any
changes.

In Photon-flaor, its style use UrlBar.Button to avoid involving tint color list.
In current design, we should not tint color for some drawable such as
Lock.

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

Comment hidden (mozreview-request)

In ToolbarDisplayLayout we use color span for Title, Domain...etc.

In Photon-flavor we use use different color in priavte mode. In
Australis-flavor, normal-mode and private-mode refers to same color, so
user should not aware anything different.

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

Jing-wei Wu [:jwu]

Comment 5

•

7 years ago

mozreview-review

Comment on attachment 8882383 [details]
Bug 1377294 - Let Site-Identity becomes theme-able

https://reviewboard.mozilla.org/r/153490/#review159258

::: mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java:166
(Diff revision 1)
>          mStop = (ImageButton) findViewById(R.id.stop);
>          mPageActionLayout = (PageActionLayout) findViewById(R.id.page_action_layout);
>      }
>  
>      @Override
> +    public void setPrivateMode(boolean isPrivate) {

Should add `super.setPrivateMode(boolean)`?

Attachment #8882383 - Flags: review?(topwu.tw) → review+

Jing-wei Wu [:jwu]

Comment 6

•

7 years ago

mozreview-review

Comment on attachment 8882384 [details]
Bug 1377294 - ToolbarDisplay layout color span supports Pirvate mode

https://reviewboard.mozilla.org/r/153492/#review159260

::: mobile/android/app/src/australis/res/values/colors.xml:111
(Diff revision 1)
>    <color name="url_bar_urltext">#AFB1B3</color>
>    <color name="url_bar_urltext_private">#777777</color>
>    <color name="url_bar_domaintext">#363B40</color>
>    <color name="url_bar_domaintext_private">#FFFFFF</color>
>    <color name="url_bar_blockedtext">#b14646</color>
> +  <color name="url_bar_blockedtext_private">#b14646</color>

Maybe just reference to `@color/url_bar_blockedtext`?

Attachment #8882384 - Flags: review?(topwu.tw) → review+

Jing-wei Wu [:jwu]

Comment 7

•

7 years ago

mozreview-review

Comment on attachment 8882382 [details]
Bug 1377294 - Let some icon in ToolbarEditLayout become Theme-able

https://reviewboard.mozilla.org/r/153488/#review159262

Attachment #8882382 - Flags: review?(topwu.tw) → review+

Jing-wei Wu [:jwu]

Comment 8

•

7 years ago

mozreview-review

Comment on attachment 8882381 [details]
Bug 1377294 - duplicate xml resource files

https://reviewboard.mozilla.org/r/153486/#review159264

Attachment #8882381 - Flags: review?(topwu.tw) → review+

Comment hidden (mozreview-request)

Comment on attachment 8882381 [details]
Bug 1377294 - duplicate xml resource files

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153486/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8882382 [details]
Bug 1377294 - Let some icon in ToolbarEditLayout become Theme-able

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153488/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8882383 [details]
Bug 1377294 - Let Site-Identity becomes theme-able

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153490/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8882384 [details]
Bug 1377294 - ToolbarDisplay layout color span supports Pirvate mode

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153492/diff/1-2/

Julian Chu [:walkingice]

Assignee

Updated

•

7 years ago

Keywords: checkin-needed

Pulsebot

Comment 13

•

7 years ago

Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cf287966c9f
duplicate xml resource files r=jwu
https://hg.mozilla.org/integration/autoland/rev/a2e429b4f101
Let some icon in ToolbarEditLayout become Theme-able r=jwu
https://hg.mozilla.org/integration/autoland/rev/3de22a3d12a9
Let Site-Identity becomes theme-able r=jwu
https://hg.mozilla.org/integration/autoland/rev/8122909722c4
ToolbarDisplay layout color span supports Pirvate mode r=jwu

Keywords: checkin-needed

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Comment 14

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/2cf287966c9f
https://hg.mozilla.org/mozilla-central/rev/a2e429b4f101
https://hg.mozilla.org/mozilla-central/rev/3de22a3d12a9
https://hg.mozilla.org/mozilla-central/rev/8122909722c4

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox56: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 56

Nevin Chen(Not active on Bugzilla)

Comment 15

•

7 years ago

mozreview-review

Comment on attachment 8882381 [details]
Bug 1377294 - duplicate xml resource files

https://reviewboard.mozilla.org/r/153486/#review160590

Attachment #8882381 - Flags: review?(cnevinchen) → review+

Nevin Chen(Not active on Bugzilla)

Comment 16

•

7 years ago

mozreview-review

Comment on attachment 8882382 [details]
Bug 1377294 - Let some icon in ToolbarEditLayout become Theme-able

https://reviewboard.mozilla.org/r/153488/#review160592

Attachment #8882382 - Flags: review?(cnevinchen) → review+

Nevin Chen(Not active on Bugzilla)

Comment 17

•

7 years ago

mozreview-review

Comment on attachment 8882383 [details]
Bug 1377294 - Let Site-Identity becomes theme-able

https://reviewboard.mozilla.org/r/153490/#review160594

Attachment #8882383 - Flags: review?(cnevinchen) → review+

Nevin Chen(Not active on Bugzilla)

Comment 18

•

7 years ago

mozreview-review

Comment on attachment 8882384 [details]
Bug 1377294 - ToolbarDisplay layout color span supports Pirvate mode

https://reviewboard.mozilla.org/r/153492/#review160596

LGTM

Attachment #8882384 - Flags: review?(cnevinchen) → review+

Kevin Brosnan [Ex-Mozilla]

Updated

•

7 years ago

Depends on: 1379503

Nobody; OK to take it and work on it

Updated

•

3 years ago

Product: Firefox for Android → Firefox for Android Graveyard

You need to log in before you can comment on or make changes to this bug.