Closed Bug 1283076 Opened 8 years ago Closed 8 years ago

Change default bookmarks for Nightly

Categories

(Firefox :: Bookmarks & History, enhancement)

Product:
Firefox
Component:
Bookmarks & History
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 52
Tracking Flags:
Tracking Status
firefox52 --- verified

People

(Reporter: pascalc, Assigned: pascalc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

bug1283076_v2.diff
24.52 KB, patch
Details | Diff | Splinter Review 
The default bookmarks are not very useful to our Nightly population as they target end users with generic information ('about us' page) or point to end-user support sites or the tour page for our UI.

For our Nightly population that is either already involved in Mozilla or that we want to involve in activities such as bug reporting, bug triaging or development, having these bookmarks pointing to bugzilla, mozregression, MDN or our Nightly blog would be more useful.
Attached patch bug1283076.diff (obsolete) — — Splinter Review 
Patch doing this:
- no change on default bookmarks for non-nightly builds
- delete browser/locales/generic/extract-bookmarks.py which is dead code
- replace firefox bookmarks by this list for Nightly:
  - Link to the Firefox Nightly blog (https://blog.nightly.mozilla.org/)
  - Link to Bugzilla https://bugzilla.mozilla.org/
  - Link to MDN https://developer.mozilla.org/
  - Link to Nightly Tester Tools addon https://addons.mozilla.org/en-US/firefox/addon/nightly-tester-tools/
  - Link to about:crashes
  - Link to Nightly IRC channel on irc.mozilla.org

The default bookmark in the bookmark toolbar which was a tour of Firefox feature is now a link to mozilla.org/contribute

This patch also:
- removes all rdf id references in links which are unused
- moves all the data-uri icons to replacement variables to avoid multiple copy-pastes and make the code easier to read
- removes the invalid mention that the file was automatically generated, it is not the case
- updates the the meta charset line for a shorter one using HTML5 syntax
- uses lowercase html
- some of the links have keywords or tags defined

I am not asking for review yet as I asked colleagues and Nightly contributors to brainstorm on which other link could be useful to add.
Assignee: nobody → pascalc
Comment on attachment 8766780 [details] [diff] [review]
bug1283076.diff

Not sure who to ask a review as this file hasn't been touched in years and not all the people listed via blame are still at Mozilla. Matt you did a review to bookmarks.in back in 2014, would you do a review for this patch please?
Attachment #8766780 - Flags: review?(mbrubeck)
Comment on attachment 8766780 [details] [diff] [review]
bug1283076.diff

I'm very sorry for the long delay on this. Passing review to Dolske since I am not a Firefox peer.
Attachment #8766780 - Flags: review?(mbrubeck) → review?(dolske)
Thanks Matt for putting my patch on the right review queue :)
Comment on attachment 8766780 [details] [diff] [review]
bug1283076.diff

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

::: browser/locales/en-US/profile/bookmarks.inc
@@ +41,5 @@
> +# Firefox Nightly links folder name
> +#define nightly_heading Firefox Nightly Resources
> +
> +# LOCALIZATION NOTE (nightly_blog):
> +# Nightly builds only, link title for https://blog.nightly.mozilla.org/

I feel compelled to note that I'm a bit wary about linking to a new blog with little activity -- best of intentions, I'm sure, but the historical tendency is for these efforts to lose steam, and then we'll be shipping a stale/forgotten bookmark.

Other ideas: link to a Nightly wiki page (that anyone can update)? Or maybe Planet itself?

@@ +62,5 @@
> +#define crashes All your crashes
> +
> +# LOCALIZATION NOTE (irc):
> +# Nightly builds only, link title for ircs://irc.mozilla.org/nightly
> +#define irc Discuss Nightly on IRC

Ugh. Our out-of-the-box experience for links like this is pretty gross. (For that matter, IRC itself is gross for new users.)

I think you'd be better off by just providing a direct Mibbit or IRCCloud link. In the rare case that someone installs their own standalone client (or has an alternate web-client), they're pretty advanced and don't need an bookmark.

::: browser/locales/generic/extract-bookmarks.py
@@ -1,1 @@
> -# This Source Code Form is subject to the terms of the Mozilla Public

Bug 1223385 removed this file in July, shortly after your patch, so you don't need to now.
Attachment #8766780 - Flags: review?(dolske) → review+
(In reply to Justin Dolske [:Dolske] from comment #5)
> Comment on attachment 8766780 [details] [diff] [review]
> bug1283076.diff
> 
> Review of attachment 8766780 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/locales/en-US/profile/bookmarks.inc
> @@ +41,5 @@
> > +# Firefox Nightly links folder name
> > +#define nightly_heading Firefox Nightly Resources
> > +
> > +# LOCALIZATION NOTE (nightly_blog):
> > +# Nightly builds only, link title for https://blog.nightly.mozilla.org/
> 
> I feel compelled to note that I'm a bit wary about linking to a new blog
> with little activity -- best of intentions, I'm sure, but the historical
> tendency is for these efforts to lose steam, and then we'll be shipping a
> stale/forgotten bookmark.


We have just created this blog a couple of months ago and are building a team with guest bloggers and several other people than myself and Marcia have already committed to blog regularly in the months to come (Mike Conley, Jean-Yves Perrier) or punctually to talk about the impact of their work on Nightly (mostly our Taiwan teams to ask for public feedback on the platform changes they work on). If at some point we would give up on the blog, we would redirect that blog to another resource so even if the bookmark were not updated at the product level, it would redirect to a maintained resource and for the end user nothing would be broken. This is also actually the case for most of the default Firefox bookmarks which already redirect to other sites and pages than the original ones.

> 
> Other ideas: link to a Nightly wiki page (that anyone can update)? Or maybe
> Planet itself?

Given that we are mostly interested in people giving direct qualified feedback, the commenting system of the blog makes more sense than a wiki to build a community around Nightly, we historically have a wiki page about Nightly (http://wiki.mozilla.org/nightly) but we are not using it for anything at the moment and don't have short term plans to extend the content there. Planet is very general and almost all posts are not related to Nightly, however I feel that Planet could be an interesting addition to the default bookmarks indeed. How about me filing a separate bug assigned to myself aiming at reevaluating the default Nightly bookmarks in 2017?


> 
> @@ +62,5 @@
> > +#define crashes All your crashes
> > +
> > +# LOCALIZATION NOTE (irc):
> > +# Nightly builds only, link title for ircs://irc.mozilla.org/nightly
> > +#define irc Discuss Nightly on IRC
> 
> Ugh. Our out-of-the-box experience for links like this is pretty gross. (For
> that matter, IRC itself is gross for new users.)
> 
> I think you'd be better off by just providing a direct Mibbit or IRCCloud
> link. In the rare case that someone installs their own standalone client (or
> has an alternate web-client), they're pretty advanced and don't need an
> bookmark.

Since we have over 1000 channels, the bookmark is about the discoverability of the specific nightly channel, not the discoverability of Moznet. We have an IRC protocol handler in Firefox and if the user does not have an external IRC client or Chatzilla installed, it already opens Mibbit by default (I tested different Nightly installations with and without an IRC client installed to check that when I wrote the patch), so wouldn't hardcoding a specific web client actually make the link less useful to the people that do have an IRC client? Given the population using Nightly is a lot more technical than on other Firefox channels, I believe that it is likely that IRC is not unknown at all to them and I wouldn't want to bypass their preferred tools. 

> 
> ::: browser/locales/generic/extract-bookmarks.py
> @@ -1,1 @@
> > -# This Source Code Form is subject to the terms of the Mozilla Public
> 
> Bug 1223385 removed this file in July, shortly after your patch, so you
> don't need to now.

Will the patch still apply because of this or should I update the patch to not have this file removal in?

Thanks!
(In reply to Pascal Chevrel:pascalc from comment #6)

> > Ugh. Our out-of-the-box experience for links like this is pretty gross. (For
> > that matter, IRC itself is gross for new users.)
> > 
> > I think you'd be better off by just providing a direct Mibbit or IRCCloud
> > link. In the rare case that someone installs their own standalone client (or
> > has an alternate web-client), they're pretty advanced and don't need an
> > bookmark.
> 
> Since we have over 1000 channels, the bookmark is about the discoverability
> of the specific nightly channel, not the discoverability of Moznet.

I meant a direct link like https://mibbit.com/?server=irc.mozilla.org&channel=%23nightly

> We have
> an IRC protocol handler in Firefox and if the user does not have an external
> IRC client or Chatzilla installed, it already opens Mibbit by default (I
> tested different Nightly installations with and without an IRC client
> installed to check that when I wrote the patch)

Both my Windows 10 and OS X machines, neither of which has ever had an IRC client installed, show the "Launch Application" dialog when clicking and irc:// link or bookmark. Mibbit is a choice, but it's not selected by default, and it's a pretty terrible dialog.

> so wouldn't hardcoding a
> specific web client actually make the link less useful to the people that do
> have an IRC client?

I would strongly suspect that the number of Nightly users who have an IRC client installed is approximately zero. (Relative to the size of the Nightly user base, and ignoring existing Mozilla community, who don't need this link because they're already on IRC :).

For that matter, IRC is sufficiently alien these days that it might be better to forget about direct irc:// or Mibbit links entirely, and instead link to a Wiki page (or blog post, or whatever) that has a brief introduction about what IRC is and how it's being used for Nightly, a >click here< to launch Mibbit, and then a blurb at the end with the server/channel info (and irc:// links) for people that already have a client.

It would be nice if we already had a page like this, but https://wiki.mozilla.org/IRC and https://developer.mozilla.org/en-US/docs/Mozilla/QA/Getting_Started_with_IRC are quite horrible.

Actually, Rust did an awesome job, as usual: https://www.rust-lang.org/en-US/community.html It's brief and immediately useful to someone looking to try things out. Wouldn't make sense to literally link to that page, but it would be a great model to copy and pare down to what you need. Please consider doing this! (You can even "Fork me on Github"! :)


> Will the patch still apply because of this or should I update the patch to
> not have this file removal in?

The patch won't apply. But you can just ignore the error, since the rest of it will. If you're not checking this in yourself, it would be good form to attach an updated patch.
Attached patch bug1283076.diff (obsolete) — — Splinter Review 
Thanks for the feedback Justin. Here is an updated patch that applies to central, with an updated IRC link to use Mibbit and and additional link to Planet Mozilla.

If that is ok for you, I'll have somebody else commit the patch as I don't have commit access.
Attachment #8766780 - Attachment is obsolete: true
Attachment #8791541 - Flags: review?(dolske)
Attachment #8791541 - Flags: review?(dolske) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/9bc9a1bb8ad6
Change default bookmarks for Nightly. r=dolske
Keywords: checkin-needed
Backed out for failing clipboard test browser_410196_paste_into_tags.js:

https://hg.mozilla.org/integration/fx-team/rev/94d6bc5adab3d63533f5c5c33df14c7d3f763859

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=9bc9a1bb8ad6a4b26496b262b8104eb1f66f3396
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=11682043&repo=fx-team

13:17:49     INFO -  34 INFO TEST-START | browser/components/places/tests/browser/browser_410196_paste_into_tags.js
13:17:50     INFO -  TEST-INFO | started process screentopng
13:17:52     INFO -  TEST-INFO | screentopng: exit 0
13:17:52     INFO -  35 INFO *** Start BrowserChrome Test Results ***
13:17:52     INFO -  36 INFO checking window state
13:17:52     INFO -  37 INFO Entering test bound
13:17:52     INFO -  38 INFO must wait for load
13:17:52     INFO -  39 INFO must wait for focus
13:17:52     INFO -  40 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | PlacesUtils in scope -
13:17:52     INFO -  41 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | PlacesUIUtils in scope -
13:17:52     INFO -  42 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | Places organizer in scope -
13:17:52     INFO -  43 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | ContentTree is in scope -
13:17:52     INFO -  44 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | A bookmark was added -
13:17:52     INFO -  45 INFO TEST-PASS | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | tag is foo -
13:17:52     INFO -  46 INFO TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_410196_paste_into_tags.js | tagNode title is foo - Got bug, expected foo
13:17:52     INFO -  Stack trace:
13:17:52     INFO -  chrome://mochikit/content/browser-test.js:test_is:960
13:17:52     INFO -  chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_410196_paste_into_tags.js:focusTag:96
13:17:52     INFO -  chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_410196_paste_into_tags.js:null:40
13:17:52     INFO -  resource://gre/modules/Task.jsm:TaskImpl_run:319
13:17:52     INFO -  resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Handler.prototype.process:937
13:17:52     INFO -  resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.walkerLoop:816
Flags: needinfo?(pascalc)
Thanks Sebastian, I'll have a look at updating the tests shortly!
Flags: needinfo?(pascalc)
Attached patch bug1283076.diff (obsolete) — — Splinter Review 
Same patch with an updated test:

./mach mochitest browser/components/places/tests/browser/browser_410196_paste_into_tags.js
...
Browser Chrome Test Summary
	Passed: 18
	Failed: 0
	Todo: 0
	Mode: e10s
Attachment #8791541 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/20d78ac1c18a
"Change default bookmarks for Nightly". r=dolske
Keywords: checkin-needed
Backed out for failing xpcshell test test_browserGlue_corrupt_nobackup_default.js:

https://hg.mozilla.org/integration/fx-team/rev/a1130cc17d4c

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=09bb9527ecc03b5cbab9ca2760e01ddb39ab4c7b
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=11709620&repo=fx-team

[task 2016-09-23T16:55:59.756589Z] 16:55:59     INFO -  TEST-START | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js
[task 2016-09-23T16:56:02.166818Z] 16:56:02  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js | xpcshell return code: 0
[task 2016-09-23T16:56:02.167090Z] 16:56:02     INFO -  TEST-INFO took 2410ms
[task 2016-09-23T16:56:02.169691Z] 16:56:02     INFO -  >>>>>>>
[task 2016-09-23T16:56:02.172138Z] 16:56:02     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2016-09-23T16:56:02.173647Z] 16:56:02     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2016-09-23T16:56:02.175139Z] 16:56:02     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2016-09-23T16:56:02.179550Z] 16:56:02     INFO -  running event loop
[task 2016-09-23T16:56:02.181303Z] 16:56:02     INFO -  browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js | Starting
[task 2016-09-23T16:56:02.182783Z] 16:56:02     INFO -  (xpcshell/head.js) | test pending (2)
[task 2016-09-23T16:56:02.184320Z] 16:56:02     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2016-09-23T16:56:02.185960Z] 16:56:02     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js |  - should have a DB now - true == true
[task 2016-09-23T16:56:02.187551Z] 16:56:02     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js |  - 2 == 2
[task 2016-09-23T16:56:02.189346Z] 16:56:02     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js |  - Expected annotation Places/SmartBookmark - true == true
[task 2016-09-23T16:56:02.192157Z] 16:56:02  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js |  - "Get Involved" == "Getting Started"
[task 2016-09-23T16:56:02.193936Z] 16:56:02     INFO -      /home/worker/workspace/build/tests/xpcshell/tests/browser/components/places/tests/unit/test_browserGlue_corrupt_nobackup_default.js:null:49
[task 2016-09-23T16:56:02.196347Z] 16:56:02     INFO -      _run_next_test@/home/worker/workspace/build/tests/xpcshell/head.js:1566:9
[task 2016-09-23T16:56:02.198186Z] 16:56:02     INFO -      do_execute_soon/<.run@/home/worker/workspace/build/tests/xpcshell/head.js:713:9
[task 2016-09-23T16:56:02.200162Z] 16:56:02     INFO -      _do_main@/home/worker/workspace/build/tests/xpcshell/head.js:210:5
[task 2016-09-23T16:56:02.204123Z] 16:56:02     INFO -      _execute_test@/home/worker/workspace/build/tests/xpcshell/head.js:545:5
[task 2016-09-23T16:56:02.206191Z] 16:56:02     INFO -      @-e:1:1

Pushed the patch to Try with full xpcshell and mochitest coverage on Linux x64: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4434438c5a0
Flags: needinfo?(pascalc)
There is also still a browser chrome failure: https://treeherder.mozilla.org/logviewer.html#?job_id=11709616&repo=fx-team

[task 2016-09-23T16:52:20.471916Z] 16:52:20     INFO -  328 INFO TEST-START | browser/components/places/tests/browser/browser_423515.js
[task 2016-09-23T16:52:21.400652Z] 16:52:21     INFO -  can move shortcut node?
[task 2016-09-23T16:52:21.410272Z] 16:52:21     INFO -  can move shortcut node?
[task 2016-09-23T16:52:21.421294Z] 16:52:21     INFO -  can move shortcut node?
[task 2016-09-23T16:52:21.432178Z] 16:52:21     INFO -  can move shortcut node?
[task 2016-09-23T16:52:21.615401Z] 16:52:21     INFO -  TEST-INFO | started process screentopng
[task 2016-09-23T16:52:22.893462Z] 16:52:22     INFO -  TEST-INFO | screentopng: exit 0
[task 2016-09-23T16:52:22.896776Z] 16:52:22     INFO -  329 INFO checking window state
[task 2016-09-23T16:52:22.899202Z] 16:52:22     INFO -  330 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | checking PlacesUtils, running in chrome context? -
[task 2016-09-23T16:52:22.901638Z] 16:52:22     INFO -  331 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | checking PlacesUIUtils, running in chrome context? -
[task 2016-09-23T16:52:22.903990Z] 16:52:22     INFO -  332 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | checking PlacesControllerDragHelper, running in chrome context? -
[task 2016-09-23T16:52:22.906117Z] 16:52:22     INFO -  333 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | confirm test root is empty -
[task 2016-09-23T16:52:22.908055Z] 16:52:22     INFO -  334 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | populate added data to the test root -
[task 2016-09-23T16:52:22.910691Z] 16:52:22     INFO -  335 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | can move regular folder node -
[task 2016-09-23T16:52:22.912736Z] 16:52:22     INFO -  336 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | populated data to the test root -
[task 2016-09-23T16:52:22.918867Z] 16:52:22     INFO -  337 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | node is folder -
[task 2016-09-23T16:52:22.921538Z] 16:52:22     INFO -  338 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | folder id and folder node item id match -
[task 2016-09-23T16:52:22.923607Z] 16:52:22     INFO -  339 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | node is folder shortcut -
[task 2016-09-23T16:52:22.926296Z] 16:52:22     INFO -  340 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut id and shortcut node item id match -
[task 2016-09-23T16:52:22.929282Z] 16:52:22     INFO -  341 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut node id and concrete id match -
[task 2016-09-23T16:52:22.931465Z] 16:52:22     INFO -  342 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | can move folder shortcut node -
[task 2016-09-23T16:52:22.933654Z] 16:52:22     INFO -  343 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | populated data to the test root -
[task 2016-09-23T16:52:22.938435Z] 16:52:22     INFO -  344 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | bookmark id and bookmark node item id match -
[task 2016-09-23T16:52:22.941191Z] 16:52:22     INFO -  345 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | query id and query node item id match -
[task 2016-09-23T16:52:22.946621Z] 16:52:22     INFO -  346 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | can move query node -
[task 2016-09-23T16:52:22.949336Z] 16:52:22     INFO -  347 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | populated data to the test root -
[task 2016-09-23T16:52:22.951553Z] 16:52:22     INFO -  348 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | Node found -
[task 2016-09-23T16:52:22.953863Z] 16:52:22     INFO -  349 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shouldn't be able to move special folder node -
[task 2016-09-23T16:52:22.957696Z] 16:52:22     INFO -  350 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut id and shortcut node item id match -
[task 2016-09-23T16:52:22.960443Z] 16:52:22     INFO -  351 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | should be able to move special folder shortcut node -
[task 2016-09-23T16:52:22.962858Z] 16:52:22     INFO -  352 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | Node found -
[task 2016-09-23T16:52:22.965885Z] 16:52:22     INFO -  353 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shouldn't be able to move special folder node -
[task 2016-09-23T16:52:22.975837Z] 16:52:22     INFO -  354 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut id and shortcut node item id match -
[task 2016-09-23T16:52:22.984300Z] 16:52:22     INFO -  355 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | should be able to move special folder shortcut node -
[task 2016-09-23T16:52:22.989802Z] 16:52:22     INFO -  356 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | Node found -
[task 2016-09-23T16:52:22.993033Z] 16:52:22     INFO -  357 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shouldn't be able to move special folder node -
[task 2016-09-23T16:52:22.996292Z] 16:52:22     INFO -  358 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut id and shortcut node item id match -
[task 2016-09-23T16:52:22.998571Z] 16:52:22     INFO -  359 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | should be able to move special folder shortcut node -
[task 2016-09-23T16:52:23.001035Z] 16:52:23     INFO -  360 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | Node found -
[task 2016-09-23T16:52:23.003610Z] 16:52:23     INFO -  361 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shouldn't be able to move special folder node -
[task 2016-09-23T16:52:23.005966Z] 16:52:23     INFO -  362 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | shortcut id and shortcut node item id match -
[task 2016-09-23T16:52:23.008401Z] 16:52:23     INFO -  363 INFO TEST-PASS | browser/components/places/tests/browser/browser_423515.js | should be able to move special folder shortcut node -
[task 2016-09-23T16:52:23.011603Z] 16:52:23     INFO -  364 INFO TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_423515.js | has new tag - Got 8, expected 1
Attached patch updated patch (obsolete) — — Splinter Review 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f54ccaea8637d0d1a96d40198c7b671564088b6
I did a try build and updated all the failing tests related to my patch, all the other failing tests are intermittents not related to bookmarks.

Hopefully this time it works!
Attachment #8794209 - Attachment is obsolete: true
Flags: needinfo?(pascalc)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95c7a910a079
Change default bookmarks for Nightly. r=dolske
Keywords: checkin-needed
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/964959527d7d
Backed out changeset 95c7a910a079 for leaking a windows in browser_passwordmgr_contextmenu.js. r=backout
Sorry, but I had to back this out again:

https://hg.mozilla.org/integration/mozilla-inbound/rev/964959527d7d25947fb2214e84ab46fc71d0cb1e

The M(cl) failure on debug builds which are also in the Try run also showed up after checkin.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=95c7a910a0794052472a111ab45a4ff8d0111925
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=36857006&repo=mozilla-inbound

14:12:38  WARNING -  TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_passwordmgr_contextmenu.js | leaked 1 window(s) until shutdown [url = chrome://passwordmgr/content/passwordManager.xul]
14:12:38  WARNING -  TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_passwordmgr_contextmenu.js | leaked 1 window(s) until shutdown [url = about:blank]

This runs after the places tests, so the environment must be changed, but there is nothing obvious in https://hg.mozilla.org/integration/mozilla-inbound/rev/95c7a910a0794052472a111ab45a4ff8d0111925

Maybe mconley or MattN can help.
Flags: needinfo?(pascalc)
Flags: needinfo?(mconley)
Flags: needinfo?(MattN+bmo)
Isn't it https://bugzilla.mozilla.org/show_bug.cgi?id=1270775 which was already filed as an intermittent?
Flags: needinfo?(pascalc)
Hey pascalc, are you able to reproduce the failure locally with this patch when running:

./mach mochitest toolkit/components/passwordmgr/test/browser/browser_passwordmgr_contextmenu.js

with a debug build?
Flags: needinfo?(mconley) → needinfo?(pascalc)
(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #22)
> Hey pascalc, are you able to reproduce the failure locally with this patch
> when running:
> 
> ./mach mochitest
> toolkit/components/passwordmgr/test/browser/browser_passwordmgr_contextmenu.
> js
> 
> with a debug build?

No, the test passes but I am on Linux and the failing builds on try Windows and OS X ones
Flags: needinfo?(pascalc)
Updated tests in patch, that seems to work now for OSX10.10 and Windows 7 debug builds:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d38601fff42cc9feabcb553e5c3d432cec31ce21

I don't think there are other failing tests related to my patch but there are tests timing out and I have little experience decyphering the test results from the try server, Sebastian could you have a look and tell me if I didn't forget to fix a test please? Thanks!
Attachment #8796491 - Attachment is obsolete: true
Flags: needinfo?(aryx.bugmail)
There don't seem to be any issues caused by this patch in the results of Try push.
Flags: needinfo?(aryx.bugmail)
Thanks Sebastian, crossing fingers and adding the checkin-needed keyword :)
Flags: needinfo?(MattN+bmo)
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db30a41cbab5
Change default bookmarks for Nightly. r=dolske
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/db30a41cbab5
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Why did this patch add tags to these bookmarks? This doesn't seem very useful, since extremely few users use tags, as far as I know, and those who do probably prefer to pick their own tags.
Flags: needinfo?(pascalc)
Flags: needinfo?(dolske)
(In reply to Dão Gottwald [:dao] from comment #29)
> Why did this patch add tags to these bookmarks? This doesn't seem very
> useful, since extremely few users use tags, as far as I know, and those who
> do probably prefer to pick their own tags.

Those are default tags to help contributors to easily get to bugzilla, get news about the Mozilla project etc.
It doesn't hurt to have bugzilla.mozilla.org proposed to a Nightly contributor if they type 'bug' in the url bar for example, on the contrary it can only make it easier for people to get to our bug reporting system. This is not for end-users but our nightly community which is technical.
Flags: needinfo?(pascalc)
(In reply to Pascal Chevrel:pascalc from comment #30)
> (In reply to Dão Gottwald [:dao] from comment #29)
> > Why did this patch add tags to these bookmarks? This doesn't seem very
> > useful, since extremely few users use tags, as far as I know, and those who
> > do probably prefer to pick their own tags.
> 
> Those are default tags to help contributors to easily get to bugzilla, get
> news about the Mozilla project etc.
> It doesn't hurt to have bugzilla.mozilla.org proposed to a Nightly
> contributor if they type 'bug' in the url bar for example, on the contrary
> it can only make it easier for people to get to our bug reporting system.

"Bug" is in the bookmark's title as well as in the URL. As far as I can tell, the bug tag adds no value as far as autocompleting in the location bar is concerned.

> This is not for end-users but our nightly community which is technical.

I still expect tags to be a rarely used feature among that population, and again those who use it will want to use their own tags.

I'd recommend getting rid of the tags. Marco, do you have an opinion?
Flags: needinfo?(mak77)
I agree. Leaving alone the fact tags are not widely used, I also think users with tags have their own categories in mind.
The use case of typing "bug" in the urlbar doesn't sound compelling, most users I know don't even refer to problems as "bugs".
Additionally, this seems it would require localized tags for each locale; issue, planet, news and bug don't cope very well with an italian localization for example.
I filed bug 1310894.
Depends on: 1310894
Flags: needinfo?(dolske)
I have reproduced this bug with nightly 50.0a1 (2016-06-29) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly 

Build ID    : 20161019030208
User Agent  : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0
[bugday-20161019]
(In reply to Maruf Rahman[:mMARUF] from comment #35)
> I have reproduced this bug with nightly 50.0a1 (2016-06-29) on Windows 7 ,
> 64 Bit !
> 
> This bug's fix is verified with latest Nightly 
> 
> Build ID    : 20161019030208
> User Agent  : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0)
> Gecko/20100101 Firefox/52.0
> [bugday-20161019]

Thanks.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
Depends on: 1346736
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: