Consolidate names of browser toolbar's element

RESOLVED FIXED in Firefox 26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: lucasr, Assigned: Margaret)

Tracking

unspecified
Firefox 26
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-fig)

Attachments

(9 attachments)

We currently have a mix of urlbar, addressbar and awesomebar in our code, layout files, and other resources. We should choose one name and apply it everywhere.
At one point we talked about wanting to avoid using the term "awesomebar". I'm in favor of "urlbar", or just "url" where appropriate (e.g. mUrlDisplayContainer).
This is a start... I wanted to rename the strings/styles that use "awesomebar", but they're currently used in the AwesomeBar code, which we haven't removed yet, so we can hold off on updating those names until we kill off the AwesomeBar code (which we can probably just go ahead and do, right?).
Assignee: nobody → margaret.leibovic
Attachment #751247 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 751247 [details] [diff] [review]
(Part 1) Rename "awesome" things "url" things in BrowserToolbar

Review of attachment 751247 [details] [diff] [review]:
-----------------------------------------------------------------

I think we should get rid of the use of addressbar too. Use urlbar everywhere?

::: mobile/android/base/BrowserToolbar.java
@@ +271,5 @@
>          mShowReader = false;
>  
>          mAnimatingEntry = false;
>  
>          mAddressBarBg = (BrowserToolbarBackground) mLayout.findViewById(R.id.address_bar_bg);

I'd like to get rid of the addressbar names too. We should probably use same name (urlbar?) everywhere, no?
Attachment #751247 - Flags: review?(lucasr.at.mozilla) → review-
Comment on attachment 751247 [details] [diff] [review]
(Part 1) Rename "awesome" things "url" things in BrowserToolbar

Asking for review again. I'm going to attach more patches to address the "we should rename everything" concern. I can fold them together to land, but I figured it's easier to keep them separate for review :)

After making these patches, I decided I'd also really like to just kill all the unused AwesomeBar code in a different bug. I understand it's useful to be able to refer to that code, but we should all have current mozilla-central trees on our machines, or we can use MXR.
Attachment #751247 - Attachment description: Rename "awesome" things "url" things in BrowserToolbar → (Part 1) Rename "awesome" things "url" things in BrowserToolbar
Attachment #751247 - Flags: review- → review?(lucasr.at.mozilla)
Attachment #754626 - Flags: review?(lucasr.at.mozilla)
Attachment #754627 - Flags: review?(lucasr.at.mozilla)
Attachment #754630 - Flags: review?(lucasr.at.mozilla)
Attachment #754632 - Flags: review?(lucasr.at.mozilla)
Attachment #754633 - Flags: review?(lucasr.at.mozilla)
Attachment #754634 - Flags: review?(lucasr.at.mozilla)
Attachment #754635 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 751247 [details] [diff] [review]
(Part 1) Rename "awesome" things "url" things in BrowserToolbar

Review of attachment 751247 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

::: mobile/android/base/BrowserToolbar.java
@@ +271,5 @@
>          mShowReader = false;
>  
>          mAnimatingEntry = false;
>  
>          mAddressBarBg = (BrowserToolbarBackground) mLayout.findViewById(R.id.address_bar_bg);

I'd like to get rid of the addressbar names too. We should probably use same name (urlbar?) everywhere, no?
Attachment #751247 - Flags: review?(lucasr.at.mozilla) → review+
Comment on attachment 754626 [details] [diff] [review]
(Part 2) Rename ic_awesomebar_* icon resources

Review of attachment 754626 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome.
Attachment #754626 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754627 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754628 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754630 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754632 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754633 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754634 - Flags: review?(lucasr.at.mozilla) → review+
Attachment #754635 - Flags: review?(lucasr.at.mozilla) → review+
Excellent patchset btw. I'd love to see patches like these more often... :-)

I suggest sending a note to our mailing list explaining the new naming convention around the URL bar.
(In reply to Lucas Rocha (:lucasr) from comment #15)
> Excellent patchset btw. I'd love to see patches like these more often... :-)

Thanks! :D

> I suggest sending a note to our mailing list explaining the new naming
> convention around the URL bar.

Good idea, I'll do that.

https://hg.mozilla.org/projects/fig/rev/173cbbdc2e8b
Whiteboard: fixed-fig
https://hg.mozilla.org/mozilla-central/rev/173cbbdc2e8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.