Closed
Bug 1197428
Opened 9 years ago
Closed 9 years ago
Remove UrlBar.ImageButton.Icon style and change inheritors
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox43 fixed)
RESOLVED
FIXED
Firefox 43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: mcomella, Assigned: tynn, Mentored)
Details
(Whiteboard: [lang=java][good first bug])
Attachments
(1 file)
3.55 KB,
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android Then, you'll need to create a patch to upload - see https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches To fix this bug, remove the style from styles.xml and change the single user in browser_toolbar to inherit from the super style (UrlBar.ImageButton.Icon). See [1] for links to these files. Why? The only place UrlBar.ImageButton.Icon is used is on edit_cancel in large*/browser_toolbar.xml, but the view is perpetually hidden – the style is effectively unused. If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC Thanks and happy coding! ^_^ [1]: https://mxr.mozilla.org/mozilla-central/search?string=imagebutton.icon&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Assignee | ||
Comment 1•9 years ago
|
||
There was also an occurrence of R.style.UrlBar_ImageButton_Icon in the PageActionLayout Java class
Attachment #8655902 -
Flags: review?(michael.l.comella)
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → tynn.dev
Reporter | ||
Comment 2•9 years ago
|
||
Comment on attachment 8655902 [details] [diff] [review] bug1197428.patch Review of attachment 8655902 [details] [diff] [review]: ----------------------------------------------------------------- Looks good – thanks Christian! I made a push to our try test servers (above). Once the push goes green, you can add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches added via checkin-needed keyword need an associated green try run. Let me know if you need help reading the results. [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8655902 -
Flags: review?(michael.l.comella) → review+
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #2) > I made a push to our try test servers (above). Oops: https://treeherder.mozilla.org/#/jobs?repo=try&revision=68f0bb7bb32c
Assignee | ||
Comment 4•9 years ago
|
||
Thank you Michael. But now I'm wondering, if this can be considered green. rc3 failed once, while it completed the second time.
Reporter | ||
Comment 5•9 years ago
|
||
Our test suites often get intermittent failures and this looks like one of them. To check, you can see if the suite has been re-run (i.e. there is another test suite with the same name on the same device) and if that was green. Additionally, if you click a failing test suite, you can look at the bar on the bottom of the screen and see if it matches any existing known intermittent failures, which are also listed on that bar. This looks good to me and is ready for check-in!
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/94a35ea51e6a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Reporter | ||
Comment 8•9 years ago
|
||
Thanks again! If you're looking for another bug, how about bug 1155860 or bug 1164879? Note that I'm not sure how difficult the latter is – our menu code is quite complex since we do our own menu loading.
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
•