1.11 KB, patch
|Details | Diff | Splinter Review|
59 bytes, text/x-review-board-request
9.43 KB, patch
|Details | Diff | Splinter Review|
(Darkspirit from bug 1377690 comment 5) > (In reply to Markus Stange [:mstange] from bug 1377690 comment 0) > > Created attachment 8882833 [details] > > testcase > > In Nightly 56 x64 20170721100159 @ Debian Testing (Linux 4.11.0-1-amd64, Radeon RX480) > (with a fresh profile with default settings, so without webrender/stylo) > my tab freezes/crashes and I get > bp-90011b08-9336-47ff-98ea-9c61f0170721 [@ IPCError-browser | ShutDownKill ] > with your attachment, > after I do a mousedown on the square (div), hold it, move it to the center of the window and release my mouse: > If I then do a rightclick, I don't get a context menu, or if I change to another tab and switch back, there is a loading circle until the tab crashes. > I am unsure where should I report this. It looks crazy to me that such a simple div could crash my Nightly. > Can someone reproduce this? (Markus Stange [:mstange] from bug 1377690 comment 6) > I can reproduce the content process hang on Mac with these steps to reproduce. Can you please file a new bug? Thanks!
Component: CSS Parsing and Computation → General
Product: Core → Firefox
Has STR: --- → yes
Keywords: crash, nightly-community
Component: General → DOM: Core & HTML
Product: Firefox → Core
Summary: tab crash: Mousedown on a bordered div and release it in the center of the window → Hang [@ nsRange::ExcludeNonSelectableNodes] when doing text selection in this testcase
It would be good to find out if this is a regression.
Keywords: crash → hang, regressionwindow-wanted
Sounds like an infinite loop?
Need a regression window here. Maybe Jet knows someone who knows about ranges/selections?
Priority: -- → P1
I think I might be hitting this too: https://perfht.ml/2u2YBeZ Was clicking/selecting text inside a github PR.
Hanging for 18 seconds inside text selection code.
Yes, thank you this profile is really helpful. Also, wow! This function can take 18 seconds. :-( Any chance you can see if you can try to come up with STRs? Or a regression range please?
Nevermind, trying to make a selection on the attached test case is enough to reproduce!
Yikes! This one's quite awful: We can get nsIContentIterator->Next() stuck on a single node. Hopefully, an easy fix? Kato-san: WDYT?
Flags: needinfo?(bugs) → needinfo?(djripper)
Tried mozregression since it's quite easy to reproduce, and it gave me bug 1351170. Local build shows the regression commit is part 1.
Keywords: regressionwindow-wanted → regression
NI Jim according to regression window in comment 9
Created attachment 8892407 [details] [diff] [review] bug source A patch to identify the offending code.
Assignee: nobody → nchen
Status: NEW → ASSIGNED
Very scary looking change. Changing the behavior we have had from 2002. I need to think about this.
Could you explain why your patch caused the regression?
In a nutshell, I think we never ran into this before because `NodeIsInTraversalRange` was not called often in practice. There are cases where we adjust the start of the range, but the new start would be after the end of the range, and `NodeIsInTraversalRange` was designed to catch these "degenerate" cases. However, before bug 1351170, we didn't adjust the range start properly, so in turn `NodeIsInTraversalRange` was not exercised often. With bug 1351170, we now regularly (and correctly) adjust the range start, but `NodeIsInTraversalRange` is failing to catch the "degenerate" cases, resulting in this regression.
Comment on attachment 8893018 [details] Bug 1383242 - Properly compare node to traversal range under different modes; https://reviewboard.mozilla.org/r/164018/#review170042 This is so über-scary patch that at least push to try before landing. The method does get called in many places via ::Init and ::PositionAt-
Attachment #8893018 - Flags: review?(bugs) → review+
Yeah, I didn't see any obviously related failures in the try run .  https://treeherder.mozilla.org/#/jobs?repo=try&revision=05c1c24b7527df4bc25ed089a37fe124f79ca016
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f9b9ffb6fea5 Properly compare node to traversal range under different modes; r=smaug
Considering the risks involved with this bug, we probably want to back out parts of bug 1351170 from 56 Beta
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You've crushed the bug. Verified fixed in Nightly 57 x64 20170805100334 @ Debian Testing. Thank you!
Status: RESOLVED → VERIFIED
Please attach a Beta patch here that takes care of comment 19 and request approval :)
status-firefox55: --- → wontfix
status-firefox56: --- → affected
status-firefox-esr52: --- → unaffected
Version: Trunk → 55 Branch
Created attachment 8895195 [details] [diff] [review] Patch for Beta Approval Request Comment [Feature/Bug causing the regression]: Bug 1351170 [User impact if declined]: Hang when selecting text on certain pages. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Patch is a simple backout of the offending commit [String changes made/needed]: None
Comment on attachment 8895195 [details] [diff] [review] Patch for Beta Partial backout to fix a hang. Let's land this for beta 2.
Attachment #8895195 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox56: affected → fixed
You need to log in before you can comment on or make changes to this bug.