Closed
Bug 1398305
Opened 7 years ago
Closed 7 years ago
Library Menu items need to be re-ordered
Categories
(Firefox :: Menus, defect, P1)
Tracking
()
People
(Reporter: abenson, Assigned: maya.messinger)
Details
(Whiteboard: [reserve-photon-structure])
Attachments
(2 files)
Order as follows: Bookmarks View Pocket List Downloads History Synced Tabs Screenshots
Updated•7 years ago
|
Whiteboard: [photon-structure] → [photon-structure] [triage]
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-structure] [triage] → [reserve-photon-structure]
Assignee | ||
Comment 2•7 years ago
|
||
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)
Comment 3•7 years ago
|
||
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)
Comment 4•7 years ago
|
||
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)
Assignee | ||
Comment 5•7 years ago
|
||
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)
Comment 6•7 years ago
|
||
(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)
Assignee | ||
Comment 7•7 years ago
|
||
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)
Comment 8•7 years ago
|
||
(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)
Comment 9•7 years ago
|
||
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.
Assignee | ||
Comment 10•7 years ago
|
||
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!
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Assignee: nobody → maya.messinger
Status: NEW → ASSIGNED
Updated•7 years ago
|
Priority: P3 → P1
Comment 12•7 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment 14•7 years ago
|
||
mozreview-review |
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+
Comment 15•7 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/93f2ac5733c4 reorder library dropdown items to spec r=Gijs
Assignee | ||
Comment 16•7 years ago
|
||
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!
Comment 17•7 years ago
|
||
(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. :-)
Updated•7 years ago
|
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Comment 18•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/93f2ac5733c4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Comment 19•7 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•