Closed
Bug 1366672
Opened 7 years ago
Closed 7 years ago
(photon) Page loading indicator
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(firefox57 verified)
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: wesley_huang, Assigned: walkingice)
References
Details
(Whiteboard: [FNC][SPT57.2][MVP])
User Story
Attachments
(7 files)
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jwu
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
cnevinchen
:
review+
|
Details |
No description provided.
Updated•7 years ago
|
User Story: (updated)
Updated•7 years ago
|
Blocks: fennec-photon-misc_ui
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → walkingice0204
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8896606 [details] Bug 1366672 - part2: Add new custom widgets https://reviewboard.mozilla.org/r/167884/#review173122 ::: mobile/android/base/moz.build:616 (Diff revision 2) > + 'drawable/DrawableWrapper', > + 'drawable/ShiftDrawable', No ".java" extenstion in both lines.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8896605 [details] Bug 1366672 - part1: add ThemedProgressBar https://reviewboard.mozilla.org/r/167882/#review173212
Attachment #8896605 -
Flags: review?(topwu.tw) → review+
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8896608 [details] Bug 1366672 - part4: use Photon design for ProgressBar https://reviewboard.mozilla.org/r/167888/#review173258 ::: mobile/android/app/src/photon/res/values/colors.xml:82 (Diff revision 3) > - <color name="progress_start">#00DCFC</color> > - <color name="progress_end">#00A2FE</color> > + <color name="normal_progress_start">#00DCFC</color> > + <color name="normal_progress_end">#00A2FE</color> > + <color name="private_progress_start">#9400FF</color> > + <color name="private_progress_end">#FF1AD9</color> There are already four colors defined above which aren't been uesd yet: photon_loading_indicator_light, photon_loading_indicator_light_private, photon_loading_indicator_dark, photon_loading_indicator_dark_private Should these new color names just reference to old ones instead of using hex code?
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8896606 [details] Bug 1366672 - part2: Add new custom widgets https://reviewboard.mozilla.org/r/167884/#review173344
Attachment #8896606 -
Flags: review?(topwu.tw) → review+
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8896605 [details] Bug 1366672 - part1: add ThemedProgressBar https://reviewboard.mozilla.org/r/167882/#review173352
Attachment #8896605 -
Flags: review?(s.kaspari) → review+
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8896606 [details] Bug 1366672 - part2: Add new custom widgets https://reviewboard.mozilla.org/r/167884/#review173354
Attachment #8896606 -
Flags: review?(s.kaspari) → review+
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8896607 [details] Bug 1366672 - Part3: use AnimatedProgressBar to replace ProgressView https://reviewboard.mozilla.org/r/167886/#review173356 ::: mobile/android/app/src/main/res/layout/gecko_app.xml:144 (Diff revision 3) > > </ViewFlipper> > > </LinearLayout> > > - <org.mozilla.gecko.toolbar.ToolbarProgressView android:id="@id/page_progress" > + <org.mozilla.gecko.widget.AnimatedProgressBar Can we remove ToolbarProgressView now?
Attachment #8896607 -
Flags: review?(s.kaspari) → review+
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8896608 [details] Bug 1366672 - part4: use Photon design for ProgressBar https://reviewboard.mozilla.org/r/167888/#review173358
Attachment #8896608 -
Flags: review?(s.kaspari) → review+
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8896609 [details] Bug 1366672 - part5: CustomTabs use AnimatedProgressBar https://reviewboard.mozilla.org/r/167890/#review173360
Attachment #8896609 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 23•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8896608 [details] Bug 1366672 - part4: use Photon design for ProgressBar https://reviewboard.mozilla.org/r/167888/#review173258 > There are already four colors defined above which aren't been uesd yet: > > photon_loading_indicator_light, > photon_loading_indicator_light_private, > photon_loading_indicator_dark, > photon_loading_indicator_dark_private > > Should these new color names just reference to old ones instead of using hex code? OK I will use exsiting ones.
Assignee | ||
Comment 24•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8896607 [details] Bug 1366672 - Part3: use AnimatedProgressBar to replace ProgressView https://reviewboard.mozilla.org/r/167886/#review173356 > Can we remove ToolbarProgressView now? I think so, there is no other class is using this.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8896608 -
Flags: review?(topwu.tw)
Attachment #8897292 -
Flags: review?(topwu.tw)
Comment 31•7 years ago
|
||
mozreview-review |
Comment on attachment 8896607 [details] Bug 1366672 - Part3: use AnimatedProgressBar to replace ProgressView https://reviewboard.mozilla.org/r/167886/#review173818
Attachment #8896607 -
Flags: review?(topwu.tw) → review+
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8896608 [details] Bug 1366672 - part4: use Photon design for ProgressBar https://reviewboard.mozilla.org/r/167888/#review173820
Attachment #8896608 -
Flags: review+
Comment 33•7 years ago
|
||
mozreview-review |
Comment on attachment 8896609 [details] Bug 1366672 - part5: CustomTabs use AnimatedProgressBar https://reviewboard.mozilla.org/r/167890/#review173822
Attachment #8896609 -
Flags: review?(topwu.tw) → review+
Comment 34•7 years ago
|
||
mozreview-review |
Comment on attachment 8897292 [details] Bug 1366672 - part6: Clean up ToolbarProgressView https://reviewboard.mozilla.org/r/168588/#review173824
Attachment #8897292 -
Flags: review?(topwu.tw) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 35•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/13affa6a9586 part1: add ThemedProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/64ff8cc86fec part2: Add new custom widgets r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/80a2c096cfa9 Part3: use AnimatedProgressBar to replace ProgressView r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/dd2474c2eaf9 part4: use Phton design for ProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/e655bcd49af8 part5: CustomTabs use AnimatedProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/df6eadeb55a9 part6 Clean up ToolbarProgressView r=jwu
Keywords: checkin-needed
Updated•7 years ago
|
Whiteboard: [FNC][SPT57.2][MVP]
I had to back these out for android linting failures like https://treeherder.mozilla.org/logviewer.html#?job_id=123237743&repo=autoland https://hg.mozilla.org/integration/autoland/rev/cfef5aec8f82a572acaafc56e50d7125de7076e1
Flags: needinfo?(walkingice0204)
Comment 37•7 years ago
|
||
FYI, the original patch also caused at least one talos performance regression: == Change summary for alert #8830 (as of August 15 2017 08:59 UTC) == Regressions: 8% remote-nytimes summary android-4-2-armv7-api15 opt 3,244.59 -> 3,505.62 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8830 However since it was backed out, talos shows the improvement/fix after the back out: == Change summary for alert #8827 (as of August 15 2017 18:38 UTC) == Improvements: 7% remote-nytimes summary android-4-2-armv7-api15 opt 3,482.93 -> 3,240.92 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8827 So just FYI if this patch is re-introduced, it may cause talos performance regressions which would need to be resolved. Thanks!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 45•7 years ago
|
||
Hi Robert, Thanks for your information. In current design we are going to add animation to progress-bar, so I can see there will be some performance drop on battery or CPU. According to our design spec, we are applying a ending-animation to progress-bar. Before hiding progress-bar, there will be 300ms ending animation. Will this effect the performance test?
Flags: needinfo?(walkingice0204) → needinfo?(rwood)
Comment 46•7 years ago
|
||
mozreview-review |
Comment on attachment 8898623 [details] Bug 1366672 - part7: Remove unused resources to make lint happy https://reviewboard.mozilla.org/r/170010/#review175188
Attachment #8898623 -
Flags: review?(cnevinchen) → review+
Comment 47•7 years ago
|
||
(In reply to Julian Chu [:walkingice] from comment #45) > Hi Robert, > > Thanks for your information. In current design we are going to add animation > to progress-bar, so I can see there will be some performance drop on battery > or CPU. > > According to our design spec, we are applying a ending-animation to > progress-bar. Before hiding progress-bar, there will be 300ms ending > animation. Will this effect the performance test? Hi Julian, thank you for your reply. I am forwrarding this on to :bc, he is familiar with this area, thanks :bc!
Flags: needinfo?(rwood) → needinfo?(bob)
Comment 48•7 years ago
|
||
If it affects the timing of the application start, page load start or page load stop, then it will appear in Autophone.
Flags: needinfo?(bob)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 60•7 years ago
|
||
The reported errors from treeherder are tier-2, and both of them have corresponding bug to track. This should be ok to check in.
Keywords: checkin-needed
Comment 61•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3403e2cea224 part1: add ThemedProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/7dfd7586ce70 part2: Add new custom widgets r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/f701c0a83fba Part3: use AnimatedProgressBar to replace ProgressView r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/82f13eb2ff6e part4: use Photon design for ProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/92d0fb788d05 part5: CustomTabs use AnimatedProgressBar r=jwu,sebastian https://hg.mozilla.org/integration/autoland/rev/664fa0772e37 part6: Clean up ToolbarProgressView r=jwu https://hg.mozilla.org/integration/autoland/rev/4da1c8e2fc43 part7: Remove unused resources to make lint happy r=nechen
Keywords: checkin-needed
Comment 62•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3403e2cea224 https://hg.mozilla.org/mozilla-central/rev/7dfd7586ce70 https://hg.mozilla.org/mozilla-central/rev/f701c0a83fba https://hg.mozilla.org/mozilla-central/rev/82f13eb2ff6e https://hg.mozilla.org/mozilla-central/rev/92d0fb788d05 https://hg.mozilla.org/mozilla-central/rev/664fa0772e37 https://hg.mozilla.org/mozilla-central/rev/4da1c8e2fc43
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 63•7 years ago
|
||
Verified as fixed on Nightly 57. The loading indicator has been changed following the design in the browser, search widget, and custom tabs.
Status: RESOLVED → VERIFIED
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.
Description
•