The default bug view has changed. See this FAQ.

Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey

VERIFIED FIXED in Firefox 14

Status

SeaMonkey
Bookmarks & History
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
seamonkey2.12
Bug Flags:
in-testsuite +

SeaMonkey Tracking Flags

(firefox14 fixed, seamonkey2.8 wontfix, seamonkey2.9 wontfix, seamonkey2.10 wontfix, seamonkey2.11 verified)

Details

Attachments

(4 attachments, 3 obsolete attachments)

24.54 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
9.92 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
15.72 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
2.12 KB, patch
mak
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Assignee)

Updated

5 years ago
Depends on: 493557
(Assignee)

Comment 1

5 years ago
Created attachment 613343 [details] [diff] [review]
(Av1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey.

(Untested) I hope i got it right...
Attachment #613343 - Flags: review?(neil)
Attachment #613343 - Flags: approval-comm-beta?
Attachment #613343 - Flags: approval-comm-aurora?
(In reply to Serge Gautherie (:sgautherie) from comment #1)
> (Untested) I hope i got it right...

Err, so who exactly should take a decision whether *this patch*, as is unreviewed and untested, should be allowed to land for Aurora/Beta?

If you want to make sure this bug gets fixed for Aurora/Beta, we have tracking flags for that (and you know it). Otherwise, I think one should only request branch approval for things that have been both reviewed (or which are trivial enough to expect passing review with only some nits, which is hardly the case here) and tested (at least so far that no obvious regressions occur, and that the patch indeed fixes the problem).

Additionally, whenever one requests branch approval now, Bugzilla should take care of adding an approval template comment. If that didn't happen in your case, I'd like to know what you did. Otherwise, please fill it in in the future instead of removing it [1]. I think it's important (read: very helpful) for those making the choice whether to grant approval or not. I don't know whether it's actually considered mandatory now, but I'm pretty sure we always required at least some risk assessment (mind you, landing something late in the Beta cycle can imply a broken release, which we certainly want to avoid, right?).

[1] Not all fields/lines are equally important there; I'm mainly concerned about the risks.
(Assignee)

Comment 3

5 years ago
Created attachment 613478 [details] [diff] [review]
(Av1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey
[Split into patches B,C,D,E]

Av1, with a typo fix.
Attachment #613343 - Attachment is obsolete: true
Attachment #613343 - Flags: review?(neil)
Attachment #613343 - Flags: approval-comm-beta?
Attachment #613343 - Flags: approval-comm-aurora?
Attachment #613478 - Flags: review?(neil)

Comment 4

5 years ago
Comment on attachment 613478 [details] [diff] [review]
(Av1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey
[Split into patches B,C,D,E]

I can't review this because the var/let and PlacesUtils changes make it impossible to tell whether there are any real changes.
Attachment #613478 - Flags: review?(neil)
(Assignee)

Comment 5

5 years ago
Created attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

(Untested) I assume/hope the s/callback/batch/ changes are not required yet.
Attachment #614028 - Flags: review?(neil)
(Assignee)

Comment 6

5 years ago
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

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

::: suite/common/places/tests/unit/test_browserGlue_smartBookmarks.js
@@ +258,5 @@
>  
> +  // Initialize SuiteGlue, but remove its listener to places-init-complete.
> +  let bg = Cc["@mozilla.org/suite/suiteglue;1"].getService(Ci.nsISuiteGlue);
> +  // Initialize Places.
> +  PlacesUtils.history;

|PlacesUtils.history;| is probably redundant in this patch (as |var hs| isn't removed yet), but I assume it doesn't hurt.
(Assignee)

Comment 7

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #6)
> |PlacesUtils.history;| is probably redundant in this patch (as |var hs|
> isn't removed yet), but I assume it doesn't hurt.

Or maybe both are needed: anyway.

Comment 8

5 years ago
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

> function rebuildSmartBookmarks() {
>-  var consoleListener = {
>+  let consoleListener = {
Drop all the var/let changes, they're not useful.

>-    print("\nTEST " + (++testIndex) + ": " + test.description);
>+    print("\nTEST: " + test.description);
Why this change?

>+  let bg = Cc["@mozilla.org/suite/suiteglue;1"].getService(Ci.nsISuiteGlue);
>+  // Initialize Places.
>+  PlacesUtils.history;
>+  // Usually places init would async notify to glue, but we want to avoid
>+  // randomness here, thus we fire the notification synchronously.
>   bg.QueryInterface(Ci.nsIObserver).observe(null, "places-init-complete", null);
Could getService(Ci.nsIObserver) perhaps?

>-    } catch(ex) { /* no version set, new profile */ }
>+    } catch(ex) {}
Why this change?
(Assignee)

Comment 9

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #8)

(It seems we two just hit the same debate quite often :-/)

> Drop all the var/let changes, they're not useful.

Can't we just copy Firefox code, to reduce (maintenance and execution) diffs?
It's just test code (and these nits are unrelated to what is tested).
And, wrt the application code, 'let' is already used in that (even both) file.

NB: I understand it may add work for you as a reviewer, but I believe it's better to get it done once than hit the diffs over and over again :-|

> >-    print("\nTEST " + (++testIndex) + ": " + test.description);
> >+    print("\nTEST: " + test.description);
> Why this change?

I assume they decided/noticed 'test.description' is enough, and (varying) numbers are not very helpful anyway.
At least, I would agree with that.

> Could getService(Ci.nsIObserver) perhaps?

Yes: to be sync'ed in a next part patch.
(You wanted to have more readable/reviewable parts... (I can understand that.))

> >-    } catch(ex) { /* no version set, new profile */ }
> >+    } catch(ex) {}
> Why this change?

I guess they thought this comment was somewhat redundant with the one before the 'try' block. (Or maybe that the 'new profile' assumption was not always right. Anyway.)
At least, I would agree with that.

(In short, I do think this patch is fine as it is.)
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

OK, so these are just syncing our test with the old version of their test?
Attachment #614028 - Flags: review?(neil) → review+
(Assignee)

Comment 11

5 years ago
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

http://hg.mozilla.org/comm-central/rev/5a705dd98711


(In reply to neil@parkwaycc.co.uk from comment #10)
> these are just syncing our test with the old version of their test?

I'm not sure what you mean ... This bug ports bug 493557 (only) and may not fully resync' these files with current Firefox files: additional resync' will be for other bugs.
Attachment #614028 - Attachment description: (Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits) → (Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits) [Checked in: Comment 11]
(Assignee)

Comment 12

5 years ago
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

http://hg.mozilla.org/comm-central/rev/5344f7011330
"Cv1 ... typo fixes for patch Bv1"
Attachment #614028 - Attachment description: (Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits) [Checked in: Comment 11] → (Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits) [Checked in: See comment 11+12]
(Assignee)

Comment 13

5 years ago
Created attachment 615224 [details] [diff] [review]
(Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...)
[Checked in: Comment 16+26]
Attachment #615224 - Flags: review?(neil)
(Assignee)

Comment 14

5 years ago
Created attachment 615225 [details] [diff] [review]
(Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes)
[Checked in: See comment 24]

(Ah, patches D and E are untested.)


(In reply to Serge Gautherie (:sgautherie) from comment #11)
> (In reply to neil@parkwaycc.co.uk from comment #10)
> > these are just syncing our test with the old version of their test?
> 
> I'm not sure what you mean ... This bug ports bug 493557 (only) and may not
> fully resync' these files with current Firefox files: additional resync'
> will be for other bugs.

Ftr, this just makes the tests "identical".
Attachment #615225 - Flags: review?(neil)

Updated

5 years ago
Attachment #615224 - Flags: review?(neil) → review+
Comment on attachment 615225 [details] [diff] [review]
(Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes)
[Checked in: See comment 24]

>+            let smartBookmark = smartBookmarks[queryId];
Unused variable. r=me with that removed.

>+          // Remove old version of the smart bookmark if it exists, since it
>+          // will be replaced in place.
>+          if (smartBookmark.itemId)
>+            PlacesUtils.bookmarks.removeItem(smartBookmark.itemId);
And, it's now pretty easy to see what the actual fix is!
(Although, it's possible that -w might have made it even easier still.)
Attachment #615225 - Flags: review?(neil) → review+
(Assignee)

Comment 16

5 years ago
Comment on attachment 615224 [details] [diff] [review]
(Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...)
[Checked in: Comment 16+26]

http://hg.mozilla.org/comm-central/rev/3fdc5f308109
Attachment #615224 - Attachment description: (Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...) → (Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...) [Checked in: Comment 16]
(Assignee)

Comment 17

5 years ago
Created attachment 615540 [details] [diff] [review]
(Fv1-FF) nsBrowserGlue.js: fix some nits

(In reply to neil@parkwaycc.co.uk from comment #15)
> >+            let smartBookmark = smartBookmarks[queryId];
> Unused variable. r=me with that removed.

Right, I forgot about that one: let's make use of it rather.
Attachment #615540 - Flags: review?(mak77)
Attachment #615540 - Flags: feedback?(neil)

Updated

5 years ago
Attachment #615540 - Flags: feedback?(neil) → feedback+
(Assignee)

Comment 18

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #16)
> http://hg.mozilla.org/comm-central/rev/3fdc5f308109

Though I didn't expect this changeset to have any effect, it caused
{
TEST-UNEXPECTED-FAIL | e:/builds/slave/test/build/xpcshell/tests/suite/common/places/tests/unit/test_421483.js | 0 != 0 - See following stack:
[...]
JS frame :: e:/builds/slave/test/build/xpcshell/tests/suite/common/places/tests/unit/test_421483.js :: run_test :: line 82
}

(up-to-date) Test code is
{
78   // TEST 2: create smart bookmarks
79   Services.prefs.setIntPref("browser.places.smartBookmarksVersion", 0);
80   gluesvc.ensurePlacesDefaultQueriesInitialized();
81   smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO);
82   do_check_neq(smartBookmarkItemIds.length, 0);
83   // check that pref has been bumped up
84   do_check_true(Services.prefs.getIntPref("browser.places.smartBookmarksVersion") > 0);
}

My guess would be the s/Services.io.newURI/NetUtil.newURI/g,
but we'll see after patch E has landed...
(Assignee)

Comment 19

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #18)

> Though I didn't expect this changeset to have any effect, it caused

It also caused:
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/places/tests/browser_library_infoBox.js | Correctly selected recently bookmarked node. - Got SeaMonkey and Mozilla, expected Recently Bookmarked
[...]
}

> but we'll see after patch E has landed...
Comment on attachment 615540 [details] [diff] [review]
(Fv1-FF) nsBrowserGlue.js: fix some nits

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

please avoid style changes where you are not actually changing code

::: browser/components/nsBrowserGlue.js
@@ +1371,1 @@
>        return;

no reason to unbrace ifs, if you want to stick to the coding stule all ifs should be braced, I don't care that much regarding that, but this is actually changing blame for no valid reason.

@@ +1425,5 @@
>              let smartBookmark = smartBookmarks[queryId];
> +            smartBookmark.itemId = itemId;
> +            smartBookmark.parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId);
> +            smartBookmark.position = PlacesUtils.bookmarks.getItemIndex(itemId);
> +          } else {

nsBrowserGlue doesn't use cuddled else, so please don't change this.
I know the coding style is for them, but that also depends on the surrounding code.

@@ +1451,1 @@
>              PlacesUtils.bookmarks.removeItem(smartBookmark.itemId);

don't unbrace
Attachment #615540 - Flags: review?(mak77)
(Assignee)

Comment 21

5 years ago
Created attachment 616394 [details] [diff] [review]
(Fv2-FF) nsBrowserGlue.js: fix 2 nits
[Checked in: Comment 23]

Fv1-FF, with comment 20 suggestion(s).
Attachment #615540 - Attachment is obsolete: true
Attachment #616394 - Flags: review?(mak77)
Comment on attachment 616394 [details] [diff] [review]
(Fv2-FF) nsBrowserGlue.js: fix 2 nits
[Checked in: Comment 23]

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

yes, this is stricter on the issue :)
Attachment #616394 - Flags: review?(mak77) → review+
(Assignee)

Comment 23

5 years ago
Comment on attachment 616394 [details] [diff] [review]
(Fv2-FF) nsBrowserGlue.js: fix 2 nits
[Checked in: Comment 23]

https://hg.mozilla.org/mozilla-central/rev/c55ce76c8dac
Attachment #616394 - Attachment description: (Fv2-FF) nsBrowserGlue.js: fix 2 nits → (Fv2-FF) nsBrowserGlue.js: fix 2 nits [Checked in: Comment 23]
(Assignee)

Comment 24

5 years ago
Comment on attachment 615225 [details] [diff] [review]
(Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes)
[Checked in: See comment 24]

http://hg.mozilla.org/comm-central/rev/d04fc59c787f
Ev1, sync'ed with patch Fv2-FF.
Attachment #615225 - Attachment description: (Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes) → (Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes) [Checked in: See comment 24]
(Assignee)

Updated

5 years ago
Attachment #613478 - Attachment description: (Av1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey → (Av1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey [Split into patches B,C,D,E]
Attachment #613478 - Attachment is obsolete: true
Comment on attachment 615225 [details] [diff] [review]
(Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes)
[Checked in: See comment 24]

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

::: suite/common/src/nsSuiteGlue.js
@@ +914,5 @@
>  
> +        let smartBookmarks = {
> +          MostVisited: {
> +            title: bundle.GetStringFromName("mostVisitedTitle"),
> +            uri: NetUtil.newURI("place:redirectsMode=" +

This (and the other two occurrences) broke us since unlike nsBrowserGlue.js, we don't have NetUtil in this context (file). You can even see that if you look at attachment 492628 [details] [diff] [review] (from the bug you ported, beginning of context).

Error: NetUtil is not defined
Source File: resource:///components/nsSuiteGlue.js
Line: 919
Pushed bustage fix with rs=Ratty over IRC:
http://hg.mozilla.org/comm-central/rev/fef14da47d5e

Aurora base had already been tagged and merged, but I got an OK from Standard8 on IRC for landing despite the closed tree:
http://hg.mozilla.org/releases/comm-aurora/rev/3b8d54c2d8ec
(Assignee)

Comment 27

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #18)
> My guess would be the s/Services.io.newURI/NetUtil.newURI/g,

I (had) made just that change in
[Mozilla/5.0 (Windows NT 5.0; rv:12.0a1) Gecko/20120126 Firefox/12.0a1 SeaMonkey/2.9a1] (custom debug build for Windows 2000)
and both xpcshell and browser-chrome tests still pass.
Unless I did something wrong, I'm rather clueless.


(In reply to Jens Hatlak (:InvisibleSmiley) from comment #25)

> Error: NetUtil is not defined
> Source File: resource:///components/nsSuiteGlue.js
> Line: 919

I wonder how to reproduce this error (with my older/custom 2.9/2.10 builds)?


(In reply to Jens Hatlak (:InvisibleSmiley) from comment #26)
> http://hg.mozilla.org/comm-central/rev/fef14da47d5e
> http://hg.mozilla.org/releases/comm-aurora/rev/3b8d54c2d8ec

Thanks for looking into this! Hope it will actually fix that regression.
(In reply to Serge Gautherie (:sgautherie) from comment #27)
> (In reply to Jens Hatlak (:InvisibleSmiley) from comment #25)
> 
> > Error: NetUtil is not defined
> > Source File: resource:///components/nsSuiteGlue.js
> > Line: 919
> 
> I wonder how to reproduce this error (with my older/custom 2.9/2.10 builds)?

No idea about older builds, but for trunk (now Aurora) prior to my fix (e.g. latest nightly as of now) you could do the following:
1. create fresh profile - see that no special Places folders are present in the BM
2. go to about:config and reset browser.places.smartBookmarksVersion
3. restart
4. open Error Console

> (In reply to Jens Hatlak (:InvisibleSmiley) from comment #26)
> > http://hg.mozilla.org/comm-central/rev/fef14da47d5e
> > http://hg.mozilla.org/releases/comm-aurora/rev/3b8d54c2d8ec
> 
> Thanks for looking into this! Hope it will actually fix that regression.

It will (I checked before checking in, just forgot to add that info to my comment). 

[I didn't check the tests, so in theory the tests could be regressed by my bustage fix, but I was really more concerned with fixing actual application code ASAP.]
(Assignee)

Comment 29

5 years ago
Was not ready to land for SeaMonkey 2.9 :-/
status-seamonkey2.9: affected → wontfix
tracking-seamonkey2.10: --- → ?
tracking-seamonkey2.9: ? → ---
(Assignee)

Updated

5 years ago
Attachment #615224 - Attachment description: (Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...) [Checked in: Comment 16] → (Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...) [Checked in: Comment 16+26]
(Assignee)

Comment 30

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #27)
> Hope it will actually fix that regression.

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1335307535.1335313861.7058.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/04/24 15:45:35
s: sea-win32-02

Fixed browser_library_infoBox.js (failure) regression :-)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1335320201.1335325922.29426.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2012/04/24 19:16:41
s: cb-seamonkey-win32-01

Fixed test_421483.js (failure) and test_browserGlue_corrupt.js (timeout) regressions. :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox14: --- → fixed
status-seamonkey2.11: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: seamonkey2.11 → seamonkey2.12
(Assignee)

Comment 31

5 years ago
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

[Approval Request Comment]
No risk (= has test).
Attachment #614028 - Flags: approval-comm-beta?
(Assignee)

Comment 32

5 years ago
Comment on attachment 615224 [details] [diff] [review]
(Dv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 2 (Var renames, ...)
[Checked in: Comment 16+26]

[Approval Request Comment]
No risk (= has test).
Attachment #615224 - Flags: approval-comm-beta?
(Assignee)

Comment 33

5 years ago
Comment on attachment 615225 [details] [diff] [review]
(Ev1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes)
[Checked in: See comment 24]

[Approval Request Comment]
No risk (= has test).
Attachment #615225 - Flags: approval-comm-beta?
(Assignee)

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-seamonkey2.11: fixed → verified
Comment on attachment 614028 [details] [diff] [review]
(Bv1) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 1 (Use PlacesUtils.jsm, Nits)
[Checked in: See comment 11+12]

This has a non-trivial (to a quick skim) hunk that won't apply. So I'm marking a-, if we can get this fixed up for beta and a quick review from a peer I'm ok with taking it within the week.
Attachment #614028 - Flags: approval-comm-beta? → approval-comm-beta-

Updated

5 years ago
Attachment #615224 - Flags: approval-comm-beta? → approval-comm-beta-

Updated

5 years ago
Attachment #615225 - Flags: approval-comm-beta? → approval-comm-beta-
(Assignee)

Comment 35

5 years ago
(In reply to Justin Wood (:Callek) from comment #34)
> I'm ok with taking it within the week.

I'll just give up on this bug.
status-seamonkey2.10: affected → wontfix
tracking-seamonkey2.10: ? → ---
You need to log in before you can comment on or make changes to this bug.