Closed Bug 1386800 Opened 7 years ago Closed 7 years ago

Photon download toolbar icon progressbar should progress from right to left on RTL builds

Categories

(Firefox :: Downloads Panel, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.1 - Aug 15
Tracking Status
firefox57 --- fixed

People

(Reporter: itiel_yn8, Assigned: sfoster)

References

(Blocks 1 open bug)

Details

(Keywords: rtl, Whiteboard: [reserve-photon-animation])

Attachments

(2 files)

Currently also on RTL builds, in-progress downloads are horizontally progressing from left to right, this should be changed so it would progress the other way.
Whiteboard: [photon-animation] [triage]
Attached image Screenshot (deleted) —
Priority: -- → P1
Flags: qe-verify+
Priority: P1 → P3
QA Contact: jwilliams
Whiteboard: [photon-animation] [triage] → [reserve-photon-animation]
Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Iteration: --- → 57.1 - Aug 15
Priority: P3 → P1
Comment on attachment 8893224 [details] Bug 1386800 - Reverse the download progress bar & fix start animation in RTL locales. https://reviewboard.mozilla.org/r/164266/#review170274 Do we need an LTR version also for indicatorArrowProgressDark? Also, why would we need to reverse the start animation?
Attachment #8893224 - Flags: review?(paolo.mozmail)
(In reply to :Paolo Amadini from comment #4) > Comment on attachment 8893224 [details] > Bug 1386800 - Reverse the download progress bar & animation in RTL locales. > > https://reviewboard.mozilla.org/r/164266/#review170274 > > Do we need an LTR version also for indicatorArrowProgressDark? We don't currently use indicatorArrowProgressDark in Photon, instead the plan is to provide a different color for --toolbarbutton-icon-fill-attention in bug 1373341. > Also, why would we need to reverse the start animation? An RTL document direction breaks the (new) start animation, and the coordinates need reversing. For reasons I hope to better understand today.
Comment on attachment 8893224 [details] Bug 1386800 - Reverse the download progress bar & fix start animation in RTL locales. https://reviewboard.mozilla.org/r/164266/#review170350
Attachment #8893224 - Flags: review+
(In reply to Sam Foster [:sfoster] from comment #5) > An RTL document direction breaks the (new) start animation, and the > coordinates need reversing. For reasons I hope to better understand today. We should probably find a better solution for the start animation, but if that's difficult we can land this workaround, as long as it's tested and working. This issue might apply to other animations as well, so it's good to investigate it regardless. Can we file a bug to remove the code and %ifdef statements for non-Photon animations?
(In reply to :Paolo Amadini from comment #7) > We should probably find a better solution for the start animation, but if > that's difficult we can land this workaround, as long as it's tested and > working. This issue might apply to other animations as well, so it's good to > investigate it regardless. We used a similar solution in other animations, e.g. * http://searchfox.org/mozilla-central/source/browser/themes/shared/toolbarbutton-icons.inc.css#61 * http://searchfox.org/mozilla-central/source/browser/themes/shared/toolbarbutton-icons.inc.css#343 I'll update the patch to use scaleX(-1) in the same way. > Can we file a bug to remove the code and %ifdef statements for non-Photon > animations? Filed bug 1387530
Ok, I finally dredged some of this back up from brain/notes when I was working on RTL in FxOS. And cross-checked with what I would expect to see in HTML vs. XUL. So we do need the RTL variant for the progressbar animation - until a time when margin-inline-start/end are supported as animatable properties. For the start notification, that is just dropping vertically with no directionality, so simply setting an explicit direction of ltr maintains the animation in both LTR and RTL.
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 2 changesets with 1 changes to 1 files (+1 heads) remote: autoland push detected remote: recorded push in pushlog remote: remote: View your changes here: remote: https://hg.mozilla.org/try/rev/ef29c172a976c18b53a18c57ed10fa1b4c404cc0 remote: https://hg.mozilla.org/try/rev/6af7bc4f65d241eae04624a0a32f50870031274d remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6af7bc4f65d241eae04624a0a32f50870031274d remote: recorded changegroup in replication log in 0.050s remote: ** unknown exception encountered, please report by visiting remote: ** https://mercurial-scm.org/wiki/BugTracker remote: ** Python 2.7.5 (default, Nov 6 2016, 00:28:07) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)] remote: ** Mercurial Distributed SCM (version 4.2.2) remote: ** Extensions loaded: blackbox, clonebundles, obsolescencehacks, pushlog, serverlog, readonly, vcsreplicator remote: Traceback (most recent call last): remote: File "/var/hg/venv_pash/bin/hg", line 45, in <module> remote: mercurial.dispatch.run() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 83, in run remote: status = (dispatch(req) or 0) & 255 remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 164, in dispatch remote: ret = _runcatch(req) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 295, in _runcatch remote: return _callcatch(ui, _runcatchfunc) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 303, in _callcatch remote: return scmutil.callcatch(ui, func) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/scmutil.py", line 146, in callcatch remote: return func() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 285, in _runcatchfunc remote: return _dispatch(req) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 912, in _dispatch remote: cmdpats, cmdoptions) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 648, in runcommand remote: ret = _runcommand(ui, options, cmd, d) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 920, in _runcommand remote: return cmdfunc() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/dispatch.py", line 909, in <lambda> remote: d = lambda: util.checksignature(func)(ui, *args, **strcmdopt) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/util.py", line 1080, in check remote: return func(*args, **kwargs) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/commands.py", line 4663, in serve remote: s.serve_forever() remote: File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 320, in serve_forever remote: return super(sshserverwrapped, self).serve_forever() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/sshserver.py", line 104, in serve_forever remote: while self.serve_one(): remote: File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 351, in serve_one remote: return super(sshserverwrapped, self).serve_one() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/sshserver.py", line 122, in serve_one remote: rsp = wireproto.dispatch(self.repo, self, cmd) remote: File "/var/hg/version-control-tools/hgext/serverlog/__init__.py", line 343, in dispatch remote: return origdispatch(repo, proto, cmd) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/extensions.py", line 231, in closure remote: return func(*(args + a), **kw) remote: File "/var/hg/version-control-tools/pylib/vcsreplicator/vcsreplicator/hgext.py", line 359, in wireprotodispatch remote: return orig(repo, proto, command) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/wireproto.py", line 570, in dispatch remote: return func(repo, proto, *args) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/wireproto.py", line 982, in unbundle remote: proto._client()) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/exchange.py", line 1777, in unbundle remote: lockandtr[2].close() remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/transaction.py", line 43, in _active remote: return func(self, *args, **kwds) remote: File "/var/hg/venv_pash/lib/python2.7/site-packages/mercurial/transaction.py", line 490, in close remote: self._postclosecallback[cat](self) remote: File "/var/hg/version-control-tools/hgext/pushlog/__init__.py", line 245, in onpostclose remote: conn.commit() remote: sqlite3.OperationalError: database is locked abort: stream ended unexpectedly (got 0 bytes, expected 4)
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/16e248610f48 Reverse the download progress bar & fix start animation in RTL locales. r=Paolo
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Depends on: 1388432
No longer depends on: 1388432
https://hg.mozilla.org/projects/date/rev/16e248610f483ae8dc3cb1d6fadffe07c501ac83 Bug 1386800 - Reverse the download progress bar & fix start animation in RTL locales. r=Paolo
I have reproduced this bug with Nightly 57.0a1 (ar) (2017-08-02) on Windows 8.1 , 64 Bit ! This bug's fix is Verified with latest Nightly (ar) ! Build ID 20170811100330 User Agent Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170809]
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: