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

If a document has multiple history entries (either by pushstate or anchor navs) only the last one is hooked into bfcache

RESOLVED FIXED in mozilla8

Status

()

Core
Document Navigation
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

unspecified
mozilla8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 11 obsolete attachments)

549 bytes, text/html
Details
38.96 KB, patch
smaug
: review+
Details | Diff | Splinter Review
84.37 KB, patch
smaug
: review+
Details | Diff | Splinter Review
8.44 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 523127 [details]
Testcase.  Follow directions in comment 0.

This is per Jonas's description on IRC (I've verified).

STR:

* In a new tab, load attached testcase.
* Click "push some state".  Note that the page now says "Got load. State=null" and "Pushed state=(random number)."
* Navigate to http://mozilla.org by typing into the location bar.
* Click and hold the back button, and select the first history entry for this tab; this entry should correspond to the initial load of the testcase.

Expected results:

* Page still says "Got load. State=null" and "Pushed state=(random number)", because we retrieved it from bfcache.

Actual results:

* Page says "Got load. State=null", presumably because we reloaded it from the network.

It looks like skipping past the most recent history entry causes us to do a full network load, rather than pull the document out of bfcache.
(Assignee)

Updated

6 years ago
Blocks: 500328
(Assignee)

Comment 1

6 years ago
Actually, I think this happens with anchor loads too.

STR:

* Type something into this bugzilla comment box.
* Click one of the anchor links (e.g. "comment 1")
* Navigate to mozilla.org using the location bar
* Go back two history entries by clicking and holding the back button

Expected results:

* Comment box still has what you typed.

Actual results:

* Comment box is empty, because we reloaded the Bugzilla page.
(Assignee)

Updated

6 years ago
Summary: Pushstate doesn't load from bfcache when skipping past the most recent history entry for a document → If a document has multiple history entries (either by pushstate or anchor navs) only the last one is hooked into bfcache
(Assignee)

Comment 2

6 years ago
Does bfcache have any mechanism to deal with two shentries sharing a content viewer (as should happen with anchor loads, as well as pushstate)?

When I try to shove content viewers into shentries after an anchor load or a pushstate, I get many assertions.
I think we need to create a separate object which is where we'd stick the cached contentviewer. All nsISHEntries that refer to the same Document would then keep a pointer to this same object.

So we'd stick the contentviewer in a shared state rather in *any* individual nsISHEntry
(Assignee)

Comment 4

6 years ago
What would be the purpose of this level of indirection, as compared to putting the contentviewer directly in each of the shentries?
It just makes it easier (and thus hopefully less error prone) to remove or add the contentviewer from all the entries.

Either would work I guess.

One advantage of the former is that it could maybe replace the id thing we are doing to figure out which SHEntries share Document.

In any case, it's totally up to you. At this point I feel like you know this code better than me.
(Assignee)

Comment 6

6 years ago
Olli, do you have any thoughts about this?  I think Jonas may be overestimating how well I understand this code.  :)

Comment 7

6 years ago
Comment 3 seems like a good way to go, yes.  And the diagnosis here (as based on the summary) is absolutely correct.

I would be surprised if we don't have an existing version of this bug for anchor nav, by the way.
(Assignee)

Updated

6 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Comment 8

6 years ago
My current plan is:

 * Create a content viewer container object.  It contains one content viewer and potentially many SHEntries.

 * Modify SHistory so it owns a list of content viewer containers.  This way, we can have a correct, global view of the live content viewers.

Comment 9

6 years ago
Why not just have the shentries own the viewer container right now?  The proposed plan doesn't sound like it would necessarily work well if entries are moved between histories or whatnot....
(Assignee)

Comment 10

6 years ago
I was trying to address the fact that SHistory is not aware that one SHEntry doesn't correspond to one content viewer.  If a page calls pushState many times, SHistory will (I think!) go ahead and evict all the content viewers, on the (incorrect) assumption that all the SHEntries created via pushState contain a unique content viewer.

I agree that moving entries between histories would seriously mess with the plan from comment 8, though.  Maybe it's OK to evict more aggressively than we need to; certainly the user would have to click "back" many times to arrive back at the page which would be in bfcache...
(Assignee)

Comment 11

6 years ago
Looking at this code, I'm kind of surprised session history works with pushState or hash navigations at all.  Most of the data in an nsSHEntry actually belongs to all the SHEntries for a given document.

Comment 12

6 years ago
Most of it is cloned when the entry is cloned, no?
(Assignee)

Comment 13

6 years ago
(In reply to comment #12)
> Most of it is cloned when the entry is cloned, no?

Yes, but much of it is mutable, and I think some subset of that actually changes.  Apparently it doesn't change enough, or most of it is relevant only to bfcache, though, since it does appear to work.  Mostly.
(Assignee)

Comment 14

6 years ago
Created attachment 530128 [details] [diff] [review]
Wip v1

This passes the test I wrote for this bug, but still needs some cleanup and likely some bugfixes.  I also need to fix sessionrestore.
(Assignee)

Comment 15

6 years ago
Created attachment 530393 [details] [diff] [review]
Part 1, v1 - Let SHEntries share content viewers

Pushing this to try.
(Assignee)

Updated

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

Comment 16

6 years ago
Comment on attachment 530393 [details] [diff] [review]
Part 1, v1 - Let SHEntries share content viewers

Looks good on try.

I'm not totally pleased with the interface here.  My goal was to avoid COM-ifying nsSHEntryShared, but I achieved that at a cost of some pretty obtuse functions.  Perhaps there's a better way to do this.  Smaug, what do you think?
Attachment #530393 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 17

6 years ago
As usual, the session restore code needs a review from zpao, but that'll change if we change the interface, so I'll wait to ask for his review.
(Assignee)

Comment 18

6 years ago
Comment on attachment 530393 [details] [diff] [review]
Part 1, v1 - Let SHEntries share content viewers

Browsing around with this build, I noticed that I'm getting the following warning when pages use pushState:

WARNING: ContentViewer exists outside gHistoryMaxViewer range: '!viewer', file ../../../../src/docshell/shistory/src/nsSHistory.cpp, line 890

Cancelling the review until I figure this out and fix it.
Attachment #530393 - Flags: review?(Olli.Pettay)

Comment 19

6 years ago
The eviction code in history makes assumptions about how far away from the current shentry it can find viewers.  Those assumptions probably need updating.
(Assignee)

Comment 20

6 years ago
Yes.  I *think* the only assumption that's invalid is that a content viewer won't exist outside such and thus a range.  The new invariant is that any content viewer which exists out of range must be equal to the content viewer of the closest SHEntry in range.  That is, you can have

 A A A B C D D D
     *     +

where * and + mark the beginning and end of the expected range, but you can't have

 A A B C D E
     *   +
(Assignee)

Comment 21

6 years ago
This is a bit tricky, because I realized that it's totally legal to have the following history:

 A A B A

(again, letters are content viewers, or documents, if you prefer).  Just call pushState three times, go back once, and refresh the page.

So I think we're going to need to build a set of the in-range content viewers and before evicting make sure that the targeted content viewer isn't in that set.
Hmm.. that seems very unfortunate and I bet will come back to bite us (I even have a feature in mind which would be basically impossible with that setup).

Could we make it so that if you have (with the ^ showing the current history position)

A A A A
    ^

and press reload, you'd end up with 

A A B B
    ^

?
(Assignee)

Comment 23

6 years ago
Well, the letters correspond to content viewers, which have a one-to-one correspondence to documents.  So I think what you're suggesting amounts to taking the fourth SHEntry and moving it from document A to document B.

I'm not convinced we can do that, because the way in which I interpret a SHEntry's state object might be affected by the document's state.

This is different from a session restore; here we're taking a SHEntry from an active document and transplanting it into another active document.

That said, I also don't like the A B A situation...
(Assignee)

Comment 24

6 years ago
Created attachment 532312 [details] [diff] [review]
Part 2, v1: Update SHistory
(Assignee)

Updated

6 years ago
Attachment #530393 - Attachment description: Patch v1 → Part 1, v1 - Let SHEntries share content viewers
(Assignee)

Updated

6 years ago
Attachment #532312 - Attachment description: Part 2: Update SHistory so it understands that SHEntries may share content viewers. → Part 2, v1: Update SHistory
(Assignee)

Comment 25

6 years ago
Pushed to try.  We'll see.

http://tbpl.mozilla.org/?tree=Try&rev=79fa6b2edcf5
(Assignee)

Comment 26

6 years ago
Created attachment 532717 [details] [diff] [review]
Part 2, v2 - Update SHistory
(Assignee)

Updated

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

Comment 27

6 years ago
Comment on attachment 530393 [details] [diff] [review]
Part 1, v1 - Let SHEntries share content viewers

This looks good on try.
Attachment #530393 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

6 years ago
Attachment #532717 - Flags: review?(Olli.Pettay)
Would you like to get this reviewed before next Aurora branching?
(Assignee)

Comment 29

6 years ago
I wouldn't be sad if this didn't make it.  In fact, given that it risks hard-to-detect regressions, it's probably better to wait until after Tuesday.
(Assignee)

Comment 30

6 years ago
Created attachment 549472 [details] [diff] [review]
Part 1, v1.1 (rebased to tip)
(Assignee)

Updated

6 years ago
Attachment #530393 - Attachment is obsolete: true
Attachment #530393 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 31

6 years ago
Created attachment 549473 [details] [diff] [review]
Part 2, v1.1 (rebased to tip)
(Assignee)

Updated

6 years ago
Attachment #532717 - Attachment is obsolete: true
Attachment #532717 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 32

6 years ago
Comment on attachment 549472 [details] [diff] [review]
Part 1, v1.1 (rebased to tip)

Smaug said he'd try to look at these soon, so I went ahead and rebased them.
Attachment #549472 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

6 years ago
Attachment #549473 - Flags: review?(Olli.Pettay)
Comment on attachment 549473 [details] [diff] [review]
Part 2, v1.1 (rebased to tip)


>+already_AddRefed<nsIContentViewer>
>+GetContentViewerForTransaction(nsISHTransaction *aTrans)
>+{
>+  nsCOMPtr<nsISHEntry> entry;
>+  aTrans->GetSHEntry(getter_AddRefs(entry));
>+  if (!entry)
>+    return nsnull;
if (expr) {
  stmt;
}

> nsSHistory::EvictWindowContentViewers(PRInt32 aFromIndex, PRInt32 aToIndex)
> {
>-  // To enforce the per SHistory object limit on cached content viewers, we
>-  // need to release all of the content viewers that are no longer in the
>-  // "window" that now ends/begins at aToIndex.  Existing content viewers
>-  // should be in the window from
>-  // aFromIndex - gHistoryMaxViewers to aFromIndex + gHistoryMaxViewers
>+  // We need to release all content viewers that are no longer in the range
>   //
>-  // We make the assumption that entries outside this range have no viewers so
>-  // that we don't have to walk the whole entire session history checking for
>-  // content viewers.
>-
>-  // This can happen on the first load of a page in a particular window
>+  //  aToIndex - gHistoryMaxViewers to aToIndex + gHistoryMaxViewers
>+  //
>+  // to ensure that this SHistory object isn't responsible for more than
>+  // gHistoryMaxViewers content viewers.
Something strange with the comment.
It looks like 'to ensure ' starts a new statement but the sentence doesn't quite make
sense (and doesn't start with capital letter)




>+  // Calculate the range that's safe from eviction.
>+  PRInt32 startSafeIndex = PR_MAX(0, aToIndex - gHistoryMaxViewers);
>+  PRInt32 endSafeIndex = PR_MIN(mLength, aToIndex + gHistoryMaxViewers);
>+
>+  LOG(("EvictWindowContentViewers(fromIndex=%d, toIndex=%d), mLength=%d. "
>+       "Safe range [%d, %d]",
>+       aFromIndex, aToIndex, mLength, startSafeIndex, endSafeIndex)); 
>+
>+  // The content viewers in range aToIndex -/+ gHistoryMaxViewers will not be
>+  // evicted.  Collect a set of them so we don't accidentally evict one of them
>+  // if it appears outside this range.
I don't understand there indexes. Why are they counted from aToIndex ?
And why +/- gHistoryMaxViewers


>+namespace {
>+
>+class SHTransAndDist
Strange class name. What does Dist mean?
Distance? 


>+    nsCOMPtr<nsISHEntryInternal> shentryInternal = do_QueryInterface(shentry);
>+    if (shentryInternal) {
>+      shentryInternal->GetLastTouched(&mLastTouched);
>+    }
>+    else {
if (expr) {
  stmt;
} else {
  stmt;
}


>+  // We now have collected all cached content viewers.  First check that we
>+  // have enough that we actually need to evict some.
>+  if ((PRInt32)transactions.Length() <= sHistoryMaxTotalViewers)
>+    return;

if (expr) {
  stmt;
}

>+
>+  // If we need to evict, sort our list of transactions and evict the largest
>+  // ones.  (We could of course get better algorithmic complexity here by using
>+  // a heap or something more clever.  But sHistoryMaxTotalViewers isn't large,
>+  // so let's not worry about it.)
>+  transactions.Sort();
>+
>+  for (PRInt32 i = transactions.Length() - 1;
>+       i >= sHistoryMaxTotalViewers; i--) {

please use --i
Attachment #549473 - Flags: review?(Olli.Pettay) → review-
Comment on attachment 549472 [details] [diff] [review]
Part 1, v1.1 (rebased to tip)

In new code, could you please use coding style
if (expr) {
  stmt;
}
>+nsSHEntryShared::RemoveFromBFCacheAsync()
>+{
>+  NS_ASSERTION(mContentViewer && mDocument,
>+               "we're not in the bfcache!");
>+
>+  // Release the reference to the contentviewer asynchronously so that the
>+  // document doesn't get nuked mid-mutation.
>+
>+  nsCOMPtr<nsIRunnable> evt =
>+      new DestroyViewerEvent(mContentViewer, mDocument);
2 extra spaces?


>+  nsresult rv = NS_DispatchToCurrentThread(evt);
>+  if (NS_FAILED(rv)) {
>+    NS_WARNING("failed to dispatch DestroyViewerEvent");
>+  }
>+  else {
if (expr) {
 stmt;
} else {
  stmt;
}


>+    // Drop presentation. Also ensures that we don't post more then one
>+    // PLEvent. Only do this if we succeeded in posting the event since
>+    // otherwise the document could be torn down mid mutation causing crashes.
Please update the code. We don't have PLEvents anymore.


>+void
>+nsSHEntryShared::CharacterDataWillChange(nsIDocument* aDocument,
>+                                   nsIContent* aContent,
>+                                   CharacterDataChangeInfo* aInfo)
Align parameters here and elsewhere.

Looks good.
Attachment #549472 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 35

6 years ago
> I don't understand there indexes. Why are they counted from aToIndex ?
> And why +/- gHistoryMaxViewers

This is a critical part, so if it's not clear to you, I need to fix the comment.

Before the patch, the invariant was that there could be no more than gHistoryMaxViewers at a time.  Since all the viewers are consecutive and the range of viewers must touch the current index (i.e., the sequence [viewers' indices] union [current index] must not skip any indices), this implies that before a navigation, all viewers are in the range aFromIndex +/- gHistoryMaxViewers.

But therefore we must make sure that after a navigation, all viewers are in the range aToIndex +/- gHistoryMaxViewers.

I (attempt) to maintain the same invariant.  Unless I did something wrong?

(To show the converse, that restricting content viewers to this range implies that we never have more than gHistoryMaxViewers: You can only get a content viewer at index X by first navigating to X.  So suppose we have viewers for indices X through gHistoryMaxViewers + X.  If we navigate to index X - 1 or gHistoryMaxViewers + X + 1, we'll evict one of our old viewers.  That's enough to show that the invariant holds.)

> please use --i

I've been getting this comment from some reviewers lately, and it's confused me.  Is there an effort to make this the standard style across the codebase?  If so, is there a reason?
(Assignee)

Comment 36

6 years ago
> Something strange with the comment. It looks like 'to ensure ' starts a new 
> statement but the sentence doesn't quite make sense (and doesn't start with 
> capital letter)

This wasn't a typo; I wanted to set off the expression aToIndex - gHistoryMaxViewers to aToIndex + gHistoryMaxViewers from the rest of the text.
(Assignee)

Comment 37

6 years ago
Created attachment 549939 [details] [diff] [review]
Part 2, v2
Attachment #549939 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

6 years ago
Attachment #549473 - Attachment is obsolete: true
Comment on attachment 549939 [details] [diff] [review]
Part 2, v2


>+  bool operator<(const TransactionAndDistance &aOther) const {
{ should be in the next line

>+  bool operator==(const TransactionAndDistance &aOther) const {
ditto

>       gTestCount++;
>       gExpected = [false, false, true, true, true, false];
>+      dump("About to run test 9.\n");
You probably don't want to keep dump()s
Attachment #549939 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 39

6 years ago
My last try run was before the rebase.  New try push: http://tbpl.mozilla.org/?tree=Try&rev=beb278605187
(Assignee)

Comment 40

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/883e581e0849
http://hg.mozilla.org/integration/mozilla-inbound/rev/cfea4859f458
Whiteboard: [inbound]
(Assignee)

Comment 41

6 years ago
Argh, there was an moth opt permaorange in the try push that I didn't notice.  Backed out from m-i.
(Assignee)

Comment 42

6 years ago
Ah, the moth orange is a real bug.

The problem is that in session restore, I use a BFCacheEntry (nsIShentryShared) as a key into a map.  As far as I can tell, js calls ToString() to generate the hash key?  In a debug build, this works great, because ToString() contains the pointer.  But in an opt build, the pointer isn't there, and we fall flat on our face.

I spoke with some people on IRC, and the consensus is that I should just make BFCacheEntry's ToString return the pointer value.  WeakMap might be a better solution, but it's buggy.

Updated

6 years ago
Whiteboard: [inbound]
(Assignee)

Comment 43

6 years ago
Created attachment 550476 [details] [diff] [review]
Part 1, v2 (fixes moth failure)
(Assignee)

Updated

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

Comment 44

6 years ago
Created attachment 550477 [details] [diff] [review]
Interdiff Part 1 v1 --> v2
(Assignee)

Comment 45

6 years ago
New try push (with part 1 v2, part 2 v2): http://tbpl.mozilla.org/?tree=Try&rev=572348ee4ac3
(Assignee)

Comment 46

6 years ago
Created attachment 550671 [details] [diff] [review]
Part 2, v2.1
Attachment #550671 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 47

6 years ago
Created attachment 550672 [details] [diff] [review]
Part 1, v2.1
Attachment #550672 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

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

Comment 48

6 years ago
Part 1, v2.1, part 2 v2 looks good on Try.  The change is to add a toString() method on BFCacheEntry which returns the this pointer value.  I also added license blocks which I'd forgotten.  I'll attach an interdiff in a moment.

Smaug, does this look OK to you?  I'd like Waldo to have a look too, since we were discussing the toString hack on IRC.

http://tbpl.mozilla.org/?tree=Try&rev=454a074d2944
(Assignee)

Comment 49

6 years ago
Comment on attachment 550671 [details] [diff] [review]
Part 2, v2.1

This patch is the same as part 2 v2; accidentally uploaded it.
Attachment #550671 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

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

Updated

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

Comment 50

6 years ago
Created attachment 550674 [details] [diff] [review]
Interdiff, part 1 v1 --> v2.1
(Assignee)

Updated

6 years ago
Attachment #550674 - Flags: review?(jeff.walden)
Could BFCacheEntry just have PRUInt64 id and use the id as hashkey?
(Assignee)

Comment 52

6 years ago
The problem is that AIUI you can't represent a PRUint64 precisely in Javascript as a number.  It could certainly have a PRUint64 ID and return that as a string, but that doesn't seem much better than returning the pointer...

Comment 53

6 years ago
> The problem is that AIUI you can't represent a PRUint64 precisely in Javascript
> as a number.

While true, that's only an issue once you hit more than 2^53 session history entries (over the process lifetime), right?  Or about 3.5 months of creating a session history entry every nanosecond (which I don't think we can actually manage to do).  No opinion on whether the integer id is better than the string approach, though.
I'd prefer integer id. It makes it clear what kind of value the id is.
Comment on attachment 550672 [details] [diff] [review]
Part 1, v2.1

So, I'd like to see .id. It is less hacky, IMO.
Attachment #550672 - Flags: review?(Olli.Pettay) → review-
(Assignee)

Comment 56

6 years ago
Will do!
(Assignee)

Comment 57

6 years ago
Created attachment 551171 [details] [diff] [review]
Part 1, v3
Attachment #551171 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

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

Comment 58

6 years ago
Created attachment 551173 [details] [diff] [review]
Interdiff part 1 v2.1 --> v3
Attachment #550674 - Attachment is obsolete: true
Attachment #550674 - Flags: review?(jeff.walden)

Updated

6 years ago
Attachment #551171 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 59

6 years ago
Looked good on try.  Fingers crossed.

http://hg.mozilla.org/integration/mozilla-inbound/rev/dc162f9bce24
http://hg.mozilla.org/integration/mozilla-inbound/rev/f9247cadf32b
Whiteboard: [inbound/]/
(Assignee)

Updated

6 years ago
Whiteboard: [inbound/]/ → [inbound]
http://hg.mozilla.org/mozilla-central/rev/dc162f9bce24
http://hg.mozilla.org/mozilla-central/rev/f9247cadf32b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Depends on: 677565
These patches ignored Fennec and busted our SessionStore
Whiteboard: [inbound]

Updated

6 years ago
Blocks: 677873
(Assignee)

Updated

6 years ago
Depends on: 678274

Updated

6 years ago
Depends on: 683777
(Assignee)

Comment 62

6 years ago
Backed out on suspicion of causing the crash in bug 683777.

We'll watch crash stats and see if this fixed the problem.

https://hg.mozilla.org/integration/mozilla-inbound/rev/66f1de1d9cba
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 63

6 years ago
Backed out the backout due to permaorange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9670f22f912d
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Depends on: 687710
(Assignee)

Comment 64

6 years ago
Backed out (temporarily) for real this time.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a34e6bed361
https://hg.mozilla.org/mozilla-central/rev/8a34e6bed361
Status: RESOLVED → REOPENED
status-firefox10: --- → affected
Resolution: FIXED → ---
(Assignee)

Comment 65

6 years ago
Re-landed with an extra null check.

https://hg.mozilla.org/integration/mozilla-inbound/rev/051b3d8ed545
https://hg.mozilla.org/mozilla-central/rev/051b3d8ed545
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
status-firefox10: affected → ---
Resolution: --- → FIXED

Updated

6 years ago
Depends on: 696973
(Assignee)

Updated

6 years ago
Depends on: 698656
You need to log in before you can comment on or make changes to this bug.