Closed Bug 1398305 Opened 3 years ago Closed 3 years ago

Library Menu items need to be re-ordered

Categories

(Firefox :: Menus, defect, P1)

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.3 - Sep 19
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: abenson, Assigned: maya.messinger)

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(2 files)

Attached image library-menu-order.png
Order as follows:

Bookmarks
View Pocket List
Downloads
History
Synced Tabs
Screenshots
Whiteboard: [photon-structure] → [photon-structure] [triage]
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-structure] [triage] → [reserve-photon-structure]
Bookmarks
View Pocket List
History
Downloads
Synced Tabs
Screenshots
Hi, I'd love to be assigned to this bug, I think it's something I can handle and I have a build of Firefox 57 ready to go! I'm kind of new, but this seems like a fix that's easy but would give me experience. I'm guessing this is a javascript file? Could someone maybe point me in the right direction? I'm in the /browser/base/content/ directory now, but are my assumptions right? Thanks!
Flags: needinfo?(jaws)
Drew has worked extensively with the Page Action menu and he would know what work is in flux with it right now. I could try to point you to the right place but I think there have been some changes recently and Drew would know most about them.
Flags: needinfo?(jaws) → needinfo?(adw)
This bug is about the Library panel, which Mike and Gijs have worked on, so I'm CC'ing them.

But this should be pretty easy to fix.  All you need to do (I think) is move appMenu-library-downloads-button above appMenu-library-remotetabs-button.  Then the ordering of buttons should match the ordering in comment 1.

appMenu-library-downloads-button is here, and appMenu-library-remotetabs-button is right above it: https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/browser/components/customizableui/content/panelUI.inc.xul#721

There may be tests that check the ordering, I'm not sure.  We can push your patch to tryserver to see if any tests fail.  Mike and Gijs might chime in, too.
Flags: needinfo?(adw)
So I went into my source code and changed the code, but in the process found out that my build of Firefox is somehow not updating even when I pull from Mercurial. I'm stuck on the Nightly build from September 6th, when I first installed everything. I've been pulling from the repository before every start, and changes are coming in, so I'm not sure where I messed up.

I still want to try to push the bug fix, because the files are being changed and this should do it. But not sure how to go about that. I added the change with hg add ., but then hg push review gives an error "skipping unreadable pattern file ç:\mozilla-source\mozilla-central'. Not sure what that means....
I don't want to run a commit command, because the file does need review.

Thanks for your help!
Flags: needinfo?(mdeboer)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(adw)
(In reply to Maya Messinger from comment #5)
> So I went into my source code and changed the code, but in the process found
> out that my build of Firefox is somehow not updating even when I pull from
> Mercurial. I'm stuck on the Nightly build from September 6th, when I first
> installed everything. I've been pulling from the repository before every
> start, and changes are coming in, so I'm not sure where I messed up.

Two questions/suggestions that might help:
1) are you using 'hg up' as well as pulling? Pulling just fetches new changesets, but it doesn't update the locally checked out code to include those changesets.
2) are you re-running ./mach build after updating the tree? Otherwise the changes in the local checked out code won't make it into the running program either.

> I still want to try to push the bug fix, because the files are being changed
> and this should do it. But not sure how to go about that. I added the change
> with hg add ., but then hg push review gives an error "skipping unreadable
> pattern file ç:\mozilla-source\mozilla-central'. Not sure what that means....

Uh, me neither...

> I don't want to run a commit command, because the file does need review.

You could try running:

hg diff > patch-1398305.txt

and attach the file here?

But also note that you can amend commits by using `hg commit --amend` (or just `hg amend`) if you have the right mercurial extensions enabled ( ./mach mercurial-setup can help you with this).

Let us know how you get on!
Flags: needinfo?(mdeboer)
Flags: needinfo?(maya.messinger)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(adw)
Ah... I have not been rebuilding Nigtly, just running it. Got it, that's embarrassing, thanks for that!

Another question I have is the protocol for patches. I'm used to git, but do I actually run "commit"? Have been opposed to that because it seems I would be committing to the source tree, not to a review repo. Been reading the "Using Mercurial" Firefox webpage, seems like I add my files to a change, and then run hg bzexport and not hg commit? But I'm not familiar with generating patch files, so I feel like I'm missing some steps.
I appreciate all of your help, I'm such a beginner and this all is basic stuff, but I'm really enjoying this work and I hope to get to a point soon where I can navigate more easily! Thanks!
Flags: needinfo?(maya.messinger) → needinfo?(gijskruitbosch+bugs)
(In reply to Maya Messinger from comment #7)
> Another question I have is the protocol for patches. I'm used to git, but do
> I actually run "commit"? Have been opposed to that because it seems I would
> be committing to the source tree, not to a review repo. Been reading the
> "Using Mercurial" Firefox webpage,

Assuming you mean this one: https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial, I would suggest reading https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/index.html instead. :-)

While you can still create patch files, if you're used to git and github pull request workflows, committing your changes and using reviewboard will be easier.

> seems like I add my files to a change,
> and then run hg bzexport and not hg commit? But I'm not familiar with
> generating patch files, so I feel like I'm missing some steps.

In git, you would commit and then push, right? And if you were using github, you would push to your fork and then create a pull request in the original repository. Reviewboard works in a similar fashion. You commit locally, and then you push to the 'review' repository, and that will create a review request automatically. You'll want to make sure your commit message starts with a bug number and ends with some review requests, e.g.:

Bug 1398305 - reorder items in the library panel, r?gijs

and then reviewboard will automatically ask me for review.

You can continue to update your commit with "hg amend" or "hg commit --amend" and repush to reviewboard, and reviewboard will deal with that (you won't need -f for pushing, like in git, and you'd be encouraged to keep changes you make as a result of review feedback as part of the main commit(s), rather than making separate "follow up" commits and then squashing them later - just fold changes in immediately).

> I appreciate all of your help, I'm such a beginner and this all is basic
> stuff, but I'm really enjoying this work and I hope to get to a point soon
> where I can navigate more easily! Thanks!

You're doing just fine, thanks for helping us out! You can also drop by IRC to ask questions if that's easier - https://wiki.mozilla.org/IRC - most of us hang out in the #fx-team channel on irc.mozilla.org .
Flags: needinfo?(gijskruitbosch+bugs)
In fact, I expect https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html is a better link in terms of how to get a change submitted.
Thanks so much, :Gijs!! Patch is pushed and awaiting review. Following your help, I tested the change on my build of Firefox and it worked, so I think should be good. Thanks for all your help!
Assignee: nobody → maya.messinger
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8908179 [details]
Bug 1398305 - reorder library dropdown items to spec

https://reviewboard.mozilla.org/r/179854/#review185048

Thanks! The actual change looks great.

However, it seems a stray configuration change made its way into this patch. If your tree has this revision checked out, the following should be enough to take the change out:

```
hg revert  -r central browser/config/mozconfig
hg amend
```

You can inspect the diff (like git show) of the current revision with: hg diff --change . ("." is kiiiind of like HEAD in git - basically it'll be the revision you have checked out in the working dir at that point).

You can then repush to mozreview and it should update the review request etc. automatically.

::: browser/config/mozconfig:10
(Diff revision 1)
>  # This file specifies the build flags for Firefox.  You can use it by adding:
>  #  . $topsrcdir/browser/config/mozconfig
>  # to the top of your mozconfig file.
>  
>  ac_add_options --enable-application=browser
> +ac_add_options --enable-artifact-builds

Can you revert this line? If you want to enable artifact builds for your local build, you can create a ".mozconfig" file at the top of your source dir, which the build system will use and hg will ignore. :-)
Attachment #8908179 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8908179 [details]
Bug 1398305 - reorder library dropdown items to spec

https://reviewboard.mozilla.org/r/179854/#review185058

Great, thanks!
Attachment #8908179 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/93f2ac5733c4
reorder library dropdown items to spec r=Gijs
Thanks so very much for helping me through this! I understand Mercurial a lot more, and think I'll be able to be a bit more independent in the future!
(In reply to Maya Messinger from comment #16)
> Thanks so very much for helping me through this! I understand Mercurial a
> lot more, and think I'll be able to be a bit more independent in the future!

Cool! Maybe you want to pick up something else next that's somewhat in the same area of code as this issue? https://bugzilla.mozilla.org/show_bug.cgi?id=1387846 should be pretty straightforward, for instance. :-)
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
https://hg.mozilla.org/mozilla-central/rev/93f2ac5733c4
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Iteration: --- → 57.3 - Sep 19
Build ID: 20170927100120
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

Verified as fixed on Firefox Nightly 58.0a1 and Firefox Beta 57.0b3 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.