Note: There are a few cases of duplicates in user autocompletion which are being worked on.

break out XPath code into separate module

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Session Restore
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dietrich, Assigned: dietrich)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 14
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

6 years ago
start moving code not directly related to session restore out into their own home.
(Assignee)

Comment 1

6 years ago
Created attachment 596230 [details] [diff] [review]
v1

move xpath generator code to a module, and rename to be more explicit.

the only change made to the xpath code itself was a s/Ci/Components.interfaces/.
Assignee: nobody → dietrich
Attachment #596230 - Flags: review?(paul)
Attachment #596230 - Flags: review?(paul) → review+
There was some similar stuff happening in bug 697903, but that stalled and included the rest of the form restoration. But then e10s concerns stalled it and it didn't pick back up.
Blocks: 697903
(Assignee)

Comment 3

6 years ago
Created attachment 601827 [details] [diff] [review]
break out XPath code into separate module (
(Assignee)

Updated

6 years ago
Attachment #596230 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 601827 [details] [diff] [review]
break out XPath code into separate module (

>diff --git a/browser/components/sessionstore/src/XPathGenerator.js b/browser/components/sessionstore/src/XPathGenerator.js
>new file mode 100644

Don't forget a license header!
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 5

6 years ago
Created attachment 601833 [details] [diff] [review]
break out XPath code into separate module (
(Assignee)

Updated

6 years ago
Attachment #601827 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Now with license header (thx zpao).
Keywords: checkin-needed
Do we expect this module to be useful for non-sessionstore code?
If so, can we put it in toolkit?
Otherwise, can the file be named SessionstoreXPathGenerator.js or put in resource:///modules/sessionstore/?
Keywords: checkin-needed
(Assignee)

Comment 8

6 years ago
Sure, this code has nothing specific to sessionstore in it anymore (note to self: other than the "sss_", which should be removed).

Where in toolkit do you suggest? We don't have /toolkit/modules yet. Could add one, I suppose.
(In reply to Dietrich Ayala (:dietrich) from comment #8)
> Sure, this code has nothing specific to sessionstore in it anymore

Yeah, but do we actually expect other code to use it?

> Where in toolkit do you suggest? We don't have /toolkit/modules yet. Could
> add one, I suppose.

toolkit/content/ contains some JS modules.
(In reply to Dão Gottwald [:dao] from comment #9)
> (In reply to Dietrich Ayala (:dietrich) from comment #8)
> > Sure, this code has nothing specific to sessionstore in it anymore
> 
> Yeah, but do we actually expect other code to use it?

Not for the forseeable future. Maybe if 697903 get's picked up again but we can always move it to a more general location then if need be.

Personally, I like resource:///modules/sessionstore/, especially as we move towards a compatmentalized sessionstore
(Assignee)

Comment 11

6 years ago
(In reply to Paul O'Shannessy [:zpao] from comment #10)
> Personally, I like resource:///modules/sessionstore/, especially as we move
> towards a compatmentalized sessionstore

Sure. Dao?
Sounds good to me.
(Assignee)

Comment 13

6 years ago
Created attachment 602155 [details] [diff] [review]
break out XPath code into separate module (
(Assignee)

Updated

6 years ago
Attachment #601833 - Attachment is obsolete: true
(Assignee)

Comment 14

6 years ago
Comment on attachment 602155 [details] [diff] [review]
break out XPath code into separate module (

* renamed to jsm
* now in modules/sessionstore/
Attachment #602155 - Flags: review?(paul)
(Assignee)

Updated

6 years ago
Blocks: 669034
Comment on attachment 602155 [details] [diff] [review]
break out XPath code into separate module (

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

::: browser/components/sessionstore/src/Makefile.in
@@ +48,5 @@
>  	nsSessionStartup.js \
>  	$(NULL)
>  
> +libs::
> +	 $(NSINSTALL) $(srcdir)/*.jsm $(FINAL_TARGET)/modules/sessionstore

Nit, you have a space after your tab in there
Attachment #602155 - Flags: review?(paul) → review+
(Assignee)

Comment 16

5 years ago
Created attachment 608545 [details] [diff] [review]
for check-in
Attachment #602155 - Attachment is obsolete: true
Attachment #608545 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6cbefc9ea3f
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/f6cbefc9ea3f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 19

5 years ago
> Yeah, but do we actually expect other code to use it?
Well yes. SeaMonkey would like to use the module too in our session restore code, rather than forking it unnecessarily.
Comment on attachment 608545 [details] [diff] [review]
for check-in

When you move this to toolkit, please use EXTRA_JS_MODULES...
(In reply to Philip Chee from comment #19)
> > Yeah, but do we actually expect other code to use it?
> Well yes. SeaMonkey would like to use the module too in our session restore
> code, rather than forking it unnecessarily.

To move this to toolkit, I think we want more demand than from a sessionstore fork. Obviously it would be easier for SeaMonkey if way more sessionstore code was shared with a stable API...
You need to log in before you can comment on or make changes to this bug.