Closed Bug 586153 Opened 9 years ago Closed 6 years ago
unique panel ID shouldn't use Date
No description provided.
FUUUUUU pressed enter and bugzilla submitted. H8. Anyway, this code is in tabbrowser.xml's addTab(): 1158 var uniqueId = "panel" + Date.now() + position; Seems like this could be prone to fail if tabs are added quickly, and run into Windows' granular timer? Seems like a monotonic counter would be safer.
Dates are the wrong source of unique IDs on all platforms for a number of reasons - a monotonic counter is the way to go.
Justin, does the unique ID have to be unique across all windows or just one window? (I'm guessing the latter.)
They likely only need to be unique in a given window (to avoid multiple elements in the document having the same ID), but given that they've also always been globally unique (or mostly so, anyways :/), we probably don't want to change that - it's possible (and perhaps even likely) that people are depending on that.
Yeah, seems safest to keep them globally unique. You could probably just append a tab counter to the window ID to do so?
(In reply to comment #5) > Yeah, seems safest to keep them globally unique. You could probably just append > a tab counter to the window ID to do so? We have window IDs? Where do I find them?
nsIDOMWindowUtils.outerWindowID is what I'm thinking of.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
OS: Windows 7 → Windows XP
I investigated bug 903834 and pushed a cset to try  that includes some debug output that checks if there's a unique ID collision. Turns out there sometimes is and it makes the whole m-bc run fail: https://tbpl.mozilla.org/php/getParsedLog.php?id=28513867&tree=Try&full=1#error0 In the real world this would probably break Firefox completely. I pushed another cset  that just adds Math.random() and that seems to be a little more collision resistant. The ideas posted here are better though.  https://tbpl.mozilla.org/?tree=Try&rev=673debddf97d  https://tbpl.mozilla.org/?tree=Try&rev=c136704aa149
Comment on attachment 811520 [details] [diff] [review] Avoid tab panel ID collisions by using a monotonic counter THREE YEARS AND MOZILLA HAS STILL NOT FIX......oh. yay! \o/
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
(In reply to Frank Yan from comment #3) > Justin, does the unique ID have to be unique across all windows or just one > window? (I'm guessing the latter.) When SeaMonkey first ported bug 179656 in bug 105885 comment 108 we didn't bother with Date.now() instead we just used a per-tabbrowser incrementing counter. Nobody seems to have complained.
Version: Trunk → 1.5.0.x Branch
Comment on attachment 811520 [details] [diff] [review] Avoid tab panel ID collisions by using a monotonic counter [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 179656 User impact if declined: Browser window might in very rare occasions get into an unusable state. Testing completed (on m-c, etc.): Landed on m-c. Risk to taking this patch (and alternatives if risky): Low risk. String or IDL/UUID changes made by this patch: None. I think this would be great if we could uplift it as far as possible. It's a safe fix that eliminates a possible issue where esp. on Window we could render a whole browser window unusable.
Comment on attachment 811520 [details] [diff] [review] Avoid tab panel ID collisions by using a monotonic counter Doesn't meet the criteria for branches past Aurora. Longstanding issue without new urgency.
Could you please give some guidance for QA to verify this? Thanks!
I don't think there is much you can do to verify this manually. I could reproduce bug 903834 quite regularly on my machine but with the patch applied I couldn't anymore. The intermittent failures from bug 903834 also don't occur anymore. Sorry, that's a rather technical issue.
You need to log in before you can comment on or make changes to this bug.