416611 - support importing tags from bookmarks.html exports (del.icio.us and others)

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, enhancement)

Description

[John Leach]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020507 Firefox/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020507 Firefox/3.0b3pre

The http://del.icio.us online bookmark service supports exporting the bookmark list to HTML.  On importing to Firefox, the tag information is ignored.

An example line from the exported file:
<pre><code>
<DT><A HREF="http://www.quagga.net/" LAST_VISIT="1202487690" ADD_DATE="1202487690" TAGS="linux,routing,zebra,quagga,bgp,ip">Quagga Software Routing Suite</A>
</code></pre>

this is imported, but the tag field in Firefox shows blank, and none of those tags exist in the "browse by tag" list.

Reproducible: Always

Comment 1

[John Leach]

Sorry, the <pre><code> tags were added by me in an attempt to ensure the example html stayed in-tact - I see that it wasn't necessary now.

Comment 2

[Marco Bonardo [:mak]]

confirming for now as an enhancement even if this appear more pertinent for an extension

Comment 3

[Mike Connor [:mconnor]]

I think this is useful, if someone wanted to add this knowledge to the import code.  Is the format del.icio.us uses somewhat standardized/documented?

Comment 4

[Stephen Ostrow]

I believe they use the format that Firefox exports its tags with.

Comment 6

[Magritte]

Going through the frustration of trying to import tagged bookmarks, I'd really like to see this added to the html import filter in Firefox.

Furthermore, it would be logical for Firefox itself to include the "TAGS=" when exporting its own bookmarks to html so that they can be re-imported into Firefox with the tag information intact. Presently the bookmark management features are sorely lacking. The only way to maintain tag information when exporting/importing bookmarks is through the .json files. However these, especially given the lack of whitespace used by Firefox and the way tree structure rather than annotation is used for storing tags, makes it essentially unreadable or modifiable by humans and relatively complex to script for.

Also, Firefox lacks any functions to import or merge .jsor: these are only accessible from the backup and restore options in the bookmark organizer and restoring a .jsor file REPLACES all existing bookmarks.

There at present Firefox lacks any ability to easily import or merge foreign bookmarks, including its own, without losing tag information. I suggest this is not an extension feature but should be basic functionality built into the Bookmark Organizer, especially now that the database driven Bookmarks are no longer easily accessible to humans or simple scripting.

Also, it does appear that Delicious' use of "TAGS=" within the hyperlink is not unique. Netvouz.com, another social bookmarking site, exports HTML files using the same method. However, Netvouz's HTML files use spaces as a tag delimiter rather than commas. It would be nice to have options to specify delimiters, but if this is deemed too complex, I would go with a format compatible with Delicious since that is the most popular social bookmarking site and converting between different HTML formatted files is much easier than dealing with json or sql files.

Comment 7

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv

Comment 8

[Mitra Ardron]

This bug is still here, and with Delicious shutting down it seems a fair number of people are trying to move bookmarks around, but failing in part because of the import/export path through Firefox is broken (losing the tags). 

This means that anyone wanting to move their bookmarks back into Firefox is essentially hosed - i.e. they end up with a flat list of a few hundred or thousand bookmarks. 

See bug #423197 for the export bug.

Comment 9

[Mike Frysinger]

Attachment: import tags from bookmarks.html

this adds rudimentary support for reading the TAGS field and adding appropriate tags.  the downside is that it adds duplicate tags with the same name.  in looking at the bookmarks API, i cant find a way to look up an existing tag by name and get its id.  if someone can point that out, i can update this patch to be much more sensible.

the string parsing on the tags field works (so that "a,b,c" creates tags for "a", "b", and "c"), but i dont know if there's a better way using the existing string API and/or helpers.

note: this is the first time ive hacked on firefox/mozilla sources, so there is probably better ways of doing this ...

Comment 10

[Marco Bonardo [:mak]]

Why are you not using nsITaggingService? tags are bookmarks just due to a wrong implementation, it won't stick to that form forever, so better going through the official API.
Also, I think we have a ParseString utility that you want to use: ParseString(const nsACString& aAstring, char aDelimiter, nsTArray<nsCString>& aArray);

Comment 11

[Mike Frysinger]

like i clearly stated, ive never worked on this code base before.  so i have no idea what API's should be used and/or are better than what i used.  i simply looked at the existing file and guessed at something that works.

i'll take a look at the API's you suggested.  thanks!

Comment 12

[Marco Bonardo [:mak]]

No problem with that, we all started hacking the same way! I was just trying to give you some hint, comments sometime end up appearing more rude than they were intended, due to lack of time.
Would be really cool if you could also try to put together a test for this, we already have some test importing bookmarks.html like http://mxr.mozilla.org/mozilla-central/source/browser/components/places/tests/unit/test_bookmarks_html.js

Comment 13

[Mike Frysinger]

converting to ParseString is trivial and makes it much cleaner.  but i dont see any code in the tree using nsITaggingService except for javascript, so i dont have any idea how to instantiate/use it in C++.

Comment 14

[Mike Frysinger]

Attachment: import tags from bookmarks.html

i dont have any clue how to work with the tagging service.  it seems to only be implemented in an IDL, takes an "nsIVariant", and has no CID defined.  trying to pass an array of strings to nsIVariant results in an error even though that's what the IDL doc string says (but it's probably referring to an array of strings as seen by javascript).

here's an updated patch that uses ParseString at least.  someone more knowledgeable in mozilla/firefox coding will have to take it from here i guess.

Comment 15

[gagarine]

I will be happy to give 20€ (via PayPal) for someone able to make a working 64bit OSX Firefox binary than I can use to import my delicious bookmark with tags. I will use it just for that and after go back to the standard Firefox version.

I will of course give the priority to Mike Frysinger because he already spend time on this issue.

Comment 16

[gagarine]

I patch if bymyself and was hable to import bookmark with tags but like say in #c9 I get duplicated tag with the same name.

Comment 17

[gagarine]

I find a way to remove duplicate: backup in json files and reimport...

Comment 18

[gagarine]

I made a binary version for OSX see https://bugzilla.mozilla.org/show_bug.cgi?id=423197#c20

Comment 19

[nodiscc]

For reference, this self-hostable web app http://sebsauvage.net/wiki/doku.php?id=php:shaarli is affected by this bug - it exports an HTML file containing lines like

<DT><A HREF="http://fluidsquares.com/" ADD_DATE="1346309344" PRIVATE="0" TAGS="webdev,html5">Fluid Squares v2. A responsive grid experiment.</A>

meant to be suitable to import in Firefox. However the TAGS= attribute is ignored at import, largely affecting the usefulness of the HTML exported file (in fact I end up with almost 1000 links totally messed up and unsorted...)

Do any of the patches suggested here actually work? If yes, could they be included in future Firefox releases? If no, is someone interested in working on this feature? Thanks

Comment 20

[eigenlicht]

Attachment: Python script that imports tags from HTML export (e.g. Delicious)

Comment 21

[eigenlicht]

Will tags for import/export be supported in the future?

IMO this really is basic functionality. All the (social) bookmarking services support import/export with tags and it's not like this would be so complex (looking at Mike Frysinger's patches).

Comment 22

[nodiscc]

@eigenlicht, thanks for the effort, your python script does not work for me (it correctly parses the first 2 bookmarks, but then fails with errors like "Could not find bookmark: 22 - https://addons.mozilla.org/fr/firefox/addon/addons-in-urlbar/").

Does the Firefox team intend to deprecate tags in bookmarks? If yes, It's understandable nobody's working on this bug (bug I politely ask you to refrain from doing that). Feedback form the developers would be appreciated.

Comment 23

[D-Jo]

Eigenlicht, nice script, but you forgot to reset the file position at the end of each cycle. You should also check that re.search returns a match, before calling group(), or you'll get an exception.

Comment 24

[D-Jo]

eigenlicht's script works if you make this change:

 # get tags for this bookmark
f.seek(0) # rewind to start of file
for line in f:
    if url in line:
        match = re.search(r'\[\s?tags\s?:[\w\s\-\,\.\&]+\]', line, re.UNICODE)
        if match is not None:
            tags = match.group()
        break


I've never used del.icio.us, but I concocted a method for adding tags to Chrome bookmarks, and I adapted the script to import them into Firefox.

My post is at http://ubuntujournal.blogspot.com/2012/06/how-to-have-bookmark-tags-in.html.

Comment 25

[D-Jo]

Attachment: Fixed Python script that imports tags from HTML export (e.g. Delicious)

Comment 26

[gumbeto]

Attachment: script for importing bookmarks with tags from delicious export

Changed regexes to follow format I got from a delicious export ATOW

Comment 27

[flominator]

Tried to use Gumbetos script but got this error:

Traceback (most recent call last):
  File "G:\14-01-09\deli.py", line 38, in <module>
    parent_id = c.execute("""SELECT id FROM moz_bookmarks WHERE type = 2 AND parent = 2 AND title = ?""", (FOLDER,)).fetchone()[0]
sqlite3.DatabaseError: file is encrypted or is not a database

Gumbeto told me to get a new version of Sqlite. So I downloaded sqlite3.dll from http://www.sqlite.org/download.html and it worked for the first four bookmarks, as you can see in the following output:

tagged bookmark with fk: 341692
tagged bookmark with fk: 341693
tagged bookmark with fk: 341694
tagged bookmark with fk: 341695
Traceback (most recent call last):
  File "deli.py", line 58, in <module>
    tags = re.search(r'TAGS="[\w\-\,\.\&\*\+\#]+"', line, re.UNICODE).group()
AttributeError: 'NoneType' object has no attribute 'group'

To me this looks like the script has some problem with one particular bookmark. Is there a way of showing which bookmark it is?

Comment 28

[gumbeto]

The latter error could not be issued by the script version I committed, which has only an 'else' on line 58. Clarifying issue with flominator over email

Comment 29

[nodiscc]

Hi, I have not been able to test this script yet, but I will certainly try it in the near future. However, and noting we sure all appreciate gumbeto's work on the script, this is still a rather hackish workaround for this bug. It requires the python runtime (not a given on Windows/OSX machines), a basic knowledge of command line, and seems to work on a trial and error basis. Many users will not be comfortable with this.

Is someone at Mozilla intersted to work on this? Can a dev chime in and tell us what amount of work is necessary to implement this? The Netscape/HTML bookmarks format is a decent format to store bookmarks with metadata (comments, private/public status, tags...), it's readable in every browser, and it could be considered a standard regarding how many bookmark sharing services/software rely on it.

Can we have an update on this or should it be buried?

Thanks

Comment 30

[gumbeto]

Please note this is not my script. I simply changed the script that was already attached and that allowed me to transfer the bookmarks as I wanted. The original regular expressions in the script didn't make much sense for the output I got from delicious, so I made a few changes to get things to work for me.

I suspect the reason why the script only works on a trial and error basis is the delicious export having different formats depending on the OS/browser/plugin version. 

The script is indeed a very hackish workaround, but it worked for me and it's better than nothing. It's clear official support would be much better though.

Comment 31

[Marco Bonardo [:mak]]

I'm working on a rewrite of the json backups, I think we may reuse the same code for html (have to verify that yet) and in such a case we may get tags support almost for free. No promises, but that's the current plan.

Comment 32

[flominator]

@Gumbeto: Definately got the wrong script in the first. Yours worked (with updated Sqlite) great. Thanks you for your support. 

One more tweak to the script that would be really nice: If a delicious link is tagged with "shortcut:foo" or "shortcut:bar" do not add them as tags, but "foo" and "bar" as keywords :)

Comment 33

[0RUBn0]

For the record, I did an export of firefox bookmarks, deleted them all and then imported the same file: Result is the same: Tags are not imported.  So, the issue is bigger than just delicious, right?

Comment 34

[Marco Bonardo [:mak]]

right, this bug is about importing tags from bookmarks.html, that is also one of our exported formats. The official format to backup/restore bookmarks in Firefox is json btw, html is supported only for third party apps.

Comment 35

[Marco Bonardo [:mak]]

The scope here is to change BookmarkHTMLUtils.jsm to import tags, it should parse the tags attribute and create them using the tagging service API on an import.
this will also need a test, that could consist in modifying the existing tests for bookmarks.html(like test_bookmarks_html.js) to also export and import a tag.

Comment 37

[Markus Kwaśnicki]

Recently I ran into the same issue. I exported my bookmarks from ownCloud 8. Its format is similar to Delicious (Netscape Bookmark File including tags). While importing back into the *Firefox Bookmark Manager* the tags are being ignored.

Googling around the web I luckily found this bug report. So I tried the most recent updated Python 2 script by @gumbeto. Although I had the same issue like @flominator considering the right sqlite3.dll file. The version from http://www.sqlite.org/download.html didn't work for me. Instead I tried the DLL file shipped with Python 3. But in the end it worked well for me.

Anyway I'm looking forward for this bug to be solved finally.

Comment 38

[0RUBn0]

I'm not quite sure where this bug stands now, but I have a page with some sql for migrating delicious bookmarks to firefox bookmarks, including tags.  I know it's delicious-specific and from one database to another (without html in-between) and not really a solution, but maybe it might help some left-over delicious people like me or help developers with reusable sql?
--> http://attic.e-motiv.net/ff-delicious/delicious-to-firefox

Comment 39

[strickland4449]

I can confirm that 1, the bug is still present in Firefox 38.05 and 2, the python script (in attachment 809785 [details]) still works at this time (June, 2015)! Today I successfully imported tags from 2802 delicious bookmarks and I'm finally free of delicious forever, praise be.

Huge thanks @eigenlicht, @gumbeto and @D-jo!

I did run into obstacles and the following details are essential:

1. You must run the script with Python 2 32 bit
2. As mentioned above, you must replace the sqlite.dll file in the python 2 dll directory with a newer version downloaded directly from the sqlite download page

(In reply to gumbeto from comment #26)
> Created attachment 809785 [details]
> script for importing bookmarks with tags from delicious export
> 
> Changed regexes to follow format I got from a delicious export ATOW

Comment 40

[Bradley Garlick]

Hello, I'm new. I'd like to be assigned to this bug, could somebody assign me please?

Comment 41

[Marco Bonardo [:mak]]

Hello, sure, feel free to ask any question

Comment 42

[Bradley Garlick]

Attachment: Rev1 - Added code to import tags

I don't have any experience in Javascript, but after a little while scanning through BookmarksHTMLUtils.html and reading the docs, I've modified the file to read in the tags from a  HTML file and add them to each new URI. I've uploaded the patch and I'd be happy if it got a review.

Comment 43

[Marco Bonardo [:mak]]

Comment on attachment 8655504 [details] [diff] [review]
Rev1 - Added code to import tags

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

the patch structure is ok, see comments below.
But we need a test too, you can find examples in toolkit/components/places/tests/unit/test_bookmarks_html.js or toolkit/components/places/tests/unit/test_384370.js
The idea is to add a bookmark with tags, export to html, remove the bookmark, import from html and check the bookmark and its tags are there.

::: toolkit/components/places/BookmarkHTMLUtils.jsm
@@ +68,5 @@
>  Cu.import("resource://gre/modules/osfile.jsm");
>  Cu.import("resource://gre/modules/FileUtils.jsm");
>  Cu.import("resource://gre/modules/PlacesUtils.jsm");
>  Cu.import("resource://gre/modules/Task.jsm");
> +var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"].getService(Ci.nsITaggingService);

you don't need this, you can use "PlacesUtils.tagging" where needed

@@ +552,5 @@
>      if (lastModified) {
>        frame.previousLastModifiedDate =
>          this._convertImportedDateToInternalDate(lastModified);
>      }
> +      

you introduced some trailing spaces here

@@ +579,5 @@
>        } catch(e) {
>        }
>      }
> +    
> +    //Adds tags to the URI, if there are any.

trailing spaces above the comment, and please add a whitespace after //

@@ +583,5 @@
> +    //Adds tags to the URI, if there are any.
> +    if (tags) {
> +      try {
> +        var tagsArray = tags.split(",");
> +        var URIToTag = NetUtil.newURI(href);

you can use frame.previousLink that it's already an nsIURI

Comment 44

[Bradley Garlick]

Attachment: Rev2 - Cleaned up code and created a test

Sorry for the delay, it took me some time to understand the automated testing system and how to write a test. Thank you for the feedback. The new patch file contains changes to the code according to your feedback, as well as a new test (and the test's addition to xpcshell.ini). The test fails without the fix and passes when the fix is applied, as expected.

Comment 45

[Marco Bonardo [:mak]]

Comment on attachment 8657705 [details] [diff] [review]
Rev2 - Cleaned up code and created a test

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

thank you, it looks mostly good, some things left to fix:

::: toolkit/components/places/BookmarkHTMLUtils.jsm
@@ +580,5 @@
>  
> +      // Adds tags to the URI, if there are any.
> +    if (tags) {
> +      try {
> +        var tagsArray = tags.split(",");

nit: please use "let" instead of var, in new code.

::: toolkit/components/places/tests/unit/test_416611.js
@@ +1,2 @@
> +const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
> +const DESCRIPTION_ANNO = "bookmarkProperties/description";

you don't need these

@@ +10,5 @@
> +let bookmarkData = [
> +  { uri: uri("http://www.toastytech.com"), tags: ["Nathan's Toasty Technology Page"] },
> +  { uri: uri("http://www.reddit.com"), tags: ["reddit: the front page of the internet"] },
> +  { uri: uri("http://www.4chan.org"), tags: ["4chan"] }
> +];

I suspect the "tags" property here should be title... So actually, why don't you merge these 2 arrays into a single one?

@@ +14,5 @@
> +];
> +
> +function run_test() {
> +  run_next_test();
> +}

an empty run_test like this (just invoking run_next_test) is also no more needed, it was needed in the past.

@@ +25,5 @@
> +  - Import bookmarks from HTML file
> +  - Check that all bookmarks are successfully imported with tags
> +*/
> +
> +add_task(function* () {

nit: give a name to the generator like function* test_import_tags() { so it's more visible in the stacks

@@ +35,5 @@
> +  // Adds bookmarks and tags to the database.
> +  for (let { uri, tags } of tagData) {
> +    PlacesUtils.tagging.tagURI(uri, tags);
> +  }
> +  

due to the current flawed design of the API, you should add bookmarks BEFORE tags. Even if it will likely work regardless, itàs a bad habit to add tags before, and as such we don't want to do that in our code.

Actually, if you merge the 2 arrays, you can walk the final array just once, add a bookmark and then tag it.

@@ +41,5 @@
> +  for (let { uri, title } of bookmarkData) {
> +    bookmarkList.push({bookmark: yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
> +                                         url: uri,
> +                                         title })});
> +  }

I think you might just use a new Set() here... add the bookmarks to the Set, then you can just iterate it using for...of (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)

@@ +52,5 @@
> +    PlacesUtils.tagging.untagURI(uri, tags);
> +  }
> +  for (let { bookmark } of bookmarkList) {
> +    yield PlacesUtils.bookmarks.remove(bookmark.guid);
> +  }

you should only need to remove bookmarks, the tagging service will notice that and remove tags automatically.

@@ +58,5 @@
> +  // Re-imports the bookmarks from the HTML file.
> +  yield BookmarkHTMLUtils.importFromFile(HTMLFile, true);
> +
> +  // Tests to ensure that the tags are still present for each bookmark URI.
> +  yield testTags();

I'd just inline the check here, no reason to have an helper since it's just invoked once

@@ +66,5 @@
> +  for (let { uri, tags } of tagData) {
> +    do_print("Test tags for " + uri.spec + ": " + tags + "\n");
> +    let foundTags = PlacesUtils.tagging.getTagsForURI(uri);
> +    Assert.equal(foundTags.length, tags.length);
> +    Assert.ok(tags.every(tag => foundTags.indexOf(tag) != -1));

now Array.prototype.includes() can be used in Nightly and will be released in Firefox 43, so you can just use foundTags.includes(tag) instead of indexOf (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes)

::: toolkit/components/places/tests/unit/xpcshell.ini
@@ +14,5 @@
>    nsDummyObserver.manifest
>    places.sparse.sqlite
>  
> +[test_000_frecency.js]
> +[test_416611.js]

please try to insert this in alphabetical order.

note we prefer to name tests in a more verbose way to clarify what they are testing, so naming this test_bookmarks_html_import_tags.js would be nicer

Comment 46

[Bradley Garlick]

Attachment: Rev3 - Test file code cleanup

Thanks again for the feedback. I have edited the code accordingly. May I ask why adding the bookmarks to a set would be preferable to pushing them onto an array in this instance? Couldn't both work just as well, or does a set provide some sort of optimisation advantage?

Comment 47

[Marco Bonardo [:mak]]

(In reply to Bradley Garlick from comment #46)
> Thanks again for the feedback. I have edited the code accordingly. May I ask
> why adding the bookmarks to a set would be preferable to pushing them onto
> an array in this instance? Couldn't both work just as well, or does a set
> provide some sort of optimisation advantage?

no strong reason, yes both would work, Set has a slightly more readable API.

Comment 48

[Marco Bonardo [:mak]]

Comment on attachment 8658757 [details] [diff] [review]
Rev3 - Test file code cleanup

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

Thanks, this looks good, just some minor nits.

Remember to add the reviewer to the commit message before setting the checkin-needed keyword

::: toolkit/components/places/tests/unit/test_bookmarks_html_import_tags.js
@@ +1,4 @@
> +let bookmarkData = [
> +  { uri: uri("http://www.toastytech.com"), title: "Nathan's Toasty Technology Page", tags: ["technology", "personal", "retro"] },
> +  { uri: uri("http://www.reddit.com"), title: "reddit: the front page of the internet", tags: ["social media", "news", "humour"] },
> +  { uri: uri("http://www.4chan.org"), title: "4chan", tags: ["discussion", "imageboard", "anime"] }

nit: we usually try to keep rows wrapped at 80 chars (partly for historical reasons, mostly for interdiff reasons and bugzilla), so it would be nice to reindent this as
{ uri: ...,
  title: ...,
  tags: ... },
{ uri: ...

@@ +21,5 @@
> +
> +  // Adds bookmarks and tags to the database.
> +  let bookmarkList = new Set();
> +  for (let { uri, title, tags } of bookmarkData) {
> +    bookmarkList.add(yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,

nit: please move parentGuid to a newline so we stay below 80 chars limit

@@ +23,5 @@
> +  let bookmarkList = new Set();
> +  for (let { uri, title, tags } of bookmarkData) {
> +    bookmarkList.add(yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
> +                                         url: uri,
> +                                         title}));

and please add a whitespace between title and } for readability

Comment 49

[Bradley Garlick]

Attachment: Rev4 - Added review to commit message, made formatting changes

Comment 50

[Marco Bonardo [:mak]]

Comment on attachment 8659927 [details] [diff] [review]
Rev4 - Added review to commit message, made formatting changes

there's no need to set the checkin? flag (it's only used for multi-part patches).
though we require a run on the Try server for checkin-needed. I just did that:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5eec36c91e7e

Comment 51

[Bradley Garlick]

(In reply to Marco Bonardo [::mak] from comment #50)
> Comment on attachment 8659927 [details] [diff] [review]
> Rev4 - Added review to commit message, made formatting changes
> 
> there's no need to set the checkin? flag (it's only used for multi-part
> patches).
> though we require a run on the Try server for checkin-needed. I just did
> that:
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5eec36c91e7e

OK, I'll bear that in mind next time. Thanks for running it on the try server. Now that that's done, do I just wait for it to get checked in? Also, if I want to push to try myself on the next bug I work on, I'll need level 1 commit access, so if I request it, would you be able to vouch for me?

Comment 52

[Marco Bonardo [:mak]]

(In reply to Bradley Garlick from comment #51)
> OK, I'll bear that in mind next time. Thanks for running it on the try
> server. Now that that's done, do I just wait for it to get checked in?

yes

> Also,
> if I want to push to try myself on the next bug I work on, I'll need level 1
> commit access, so if I request it, would you be able to vouch for me?

Sure I would.

Try is showing some failures but they all look related to a bad changeset in fx-team and not related to this patch.

Comment 53

[Pulsebot]

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

Comment 54

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/83d4e623d476