Closed Bug 1457322 Opened 7 years ago Closed 7 years ago

Moving mouse vertically off inspector results in blue page

Categories

(DevTools :: Inspector, defect, P2)

Product:
DevTools
Component:
Inspector
Version:
61 Branch
Type:
defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61+ fixed, firefox62+ fixed)

Status:
RESOLVED DUPLICATE of bug 1461231
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + fixed
firefox62 + fixed

People

(Reporter: shobson, Assigned: bradwerth)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

Screen Shot 2018-04-26 at 15.29.05.png
1.73 MB, image/png
Details
box-model-trying-to-hide.gif
429.46 KB, image/gif
Details
With the DOM Inspector open, if I move my mouse from the inspector panel up onto the page, it highlights the last element hovered and paints it blue. Since this is the HTML element the result is a blue page. It makes it hard to see the page and I've usually moved my mouse up to try to interact with the page. It took me a while to figure out what was happening and how to avoid it.
Thanks for filing. This is a regression that we need to fix now as it can be very annoying to get rid of the highlighter.
Component: Developer Tools → Developer Tools: Inspector
Priority: -- → P2
I could swear I had seen the problem earlier this week too, but now I can't reproduce it any longer. If you still see the problem locally, could you please try to provide a list of detailed steps to reproduce? Maybe it only happens when you move your mouse a certain way? Maybe you need to wait for a little bit? Stuff Also, it would really help if you could check which exact version this happens on. I'm wondering if I may have a slightly more recent 61 than you, and maybe the problem has gone away? (crossed fingers). Is this on macOS? Thanks Stephanie!
Flags: needinfo?(shobson)
Happening for me on 61.0a1. Steps to reproduce: 1) open this page: https://www.mozilla.org/zh-TW/firefox/whatsnew/ - I suggest this one because it has a really long HTML element 2) open the element inspector 3) use the inspector to inspect the Firefox logo - the goal here is to have the bright blue selection line on a small element that does not take up the entire page - make sure your browser window is big enough that you can see the <html> element inside the inspector 4) move the mouse up to the website I don't get it if I go unnaturally fast. I generally get it going at what I consider a normal pace (it happened a lot to me during regular use yesterday). But not always. I can trigger it consistently by going slowly. If you're trying that you can intentionally hover (but not click) the HTML element before moving the mouse off the inspector.
Flags: needinfo?(shobson)
Oh yes, on MacOS.
Thanks for the information Stephanie. I'm running mozregression at the moment to find out what caused this issue. I'll update the bug once I know more.
Here's what I got: Last good revision: c783649db622b222fbe2477998006cb3d20a1ff9 First bad revision: 229be2ad5ee3a60ddc31568792725bee2a6564dc Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c783649db622b222fbe2477998006cb3d20a1ff9&tochange=229be2ad5ee3a60ddc31568792725bee2a6564dc which points to bug 1453668. I'll need to verify locally that that's correct.
So far I'm clueless as to what could be the root cause for this. Looking at the DevTools code, everything seems to be functioning properly. The mouseout event is triggered, the request to hide the highlighter is sent and handled, and then we attempt to add the "hidden" attribute to the SVG element of the highlighter (which should hide it). I was able to confirm that this code runs: https://searchfox.org/mozilla-central/rev/08df4e6e11284186d477d7e5b0ae48483ecc979c/devtools/server/actors/highlighters/box-model.js#393-395 but that the highlighter does not get hidden. Oddly, this does not seem to happen when the tested element is small, in the page. If it gets taller (like more than 90% of the viewport's height) then the problem starts to occur.
I'll try to figure this out.
Assignee: nobody → bwerth
I can reproduce on macOS. For me, the easiest way to reproduce it is these steps: 1) select one item in the Inspector 2) hover over another item in the inspector 3) slowly move the cursor to the LEFT off the window. If the highlighter remains on (it doesn't for me on every element), then it will stay on as you move the cursor back in the window in the content area, and stay that way until you trigger a cursor change by moving the cursor over a link. So far, on the https://www.mozilla.org/zh-TW/firefox/whatsnew/ page, I can get this to happen on the html element, the body element, and the first div element (with id "outer-wrapper"). Once I get to children of the div, the highlighter consistently disappears as I move the cursor to the left. What those elements have in common is a small indentation level (either none or one step). On other sites, I can get to happen on more deeply-nested elements. I'll keep investigating.
Hmmm... I can reproduce in release Nightly, but not in my local Nightly build. That makes it hard to determine causes and to test fixes. I'll try debugging the JS stack/behavior on release Nightly.
(In reply to Patrick Brosset <:pbro> from comment #7) > I was able to confirm that this code runs: > https://searchfox.org/mozilla-central/rev/ > 08df4e6e11284186d477d7e5b0ae48483ecc979c/devtools/server/actors/highlighters/ > box-model.js#393-395 > but that the highlighter does not get hidden. I can't replicate this. As I debug it, it seems that the packet is prepared, sent(?), but never received and this code is NOT called in the failure case. I don't know a lot about the transport protocol, but I'm trying to figure out the difference in the code paths between my local Nightly (which works correctly and hides the box model) and my release Nightly (which exhibits the failure).
(In reply to Brad Werth [:bradwerth] from comment #11) > I can't replicate this. As I debug it, it seems that the packet is prepared, > sent(?), but never received and this code is NOT called in the failure case. > I don't know a lot about the transport protocol, but I'm trying to figure > out the difference in the code paths between my local Nightly (which works > correctly and hides the box model) and my release Nightly (which exhibits > the failure). I've traced as far as https://searchfox.org/mozilla-central/source/devtools/shared/transport/transport.js#775 where it appears that a packet { to : "server1.conn0.child1/highlighter27", type : "hideBoxModel" } is passed to the message manager, but I can't step into the sendAsyncMessage() function to see what happens to it. Jason, I'm needinfo-ing you for insight about two things: 1) Is there a way for the debugger to step into the workings of the message manager sendAsyncMessage() function? 2) What are the best practices for debugging situations where a message is sent from a client actor but not received by the server actor?
Flags: needinfo?(jlaster)
Hi, sorry I'm not sure. I would usually set breakpoints in those cases and then track the flow that way. > not received by the server actor? that is difficult, it is hard to pause in the server. I usually just do `dump` statements
Flags: needinfo?(jlaster)
(In reply to Brad Werth [:bradwerth] from comment #11) > (In reply to Patrick Brosset <:pbro> from comment #7) > > I was able to confirm that this code runs: > > https://searchfox.org/mozilla-central/rev/ > > 08df4e6e11284186d477d7e5b0ae48483ecc979c/devtools/server/actors/highlighters/ > > box-model.js#393-395 > > but that the highlighter does not get hidden. > > I can't replicate this. As I debug it, it seems that the packet is prepared, > sent(?), but never received and this code is NOT called in the failure case. I can confirm that for me, locally, this is *not* a protocol problem. I added a simple console.log("hiding") to the _hideBoxModel method of the box-model highlighter class and I can see it in the browser console (ctrl+shift+J) in the failure cases too.
(In reply to Patrick Brosset <:pbro> from comment #14) > I can confirm that for me, locally, this is *not* a protocol problem. I > added a simple console.log("hiding") to the _hideBoxModel method of the > box-model highlighter class and I can see it in the browser console > (ctrl+shift+J) in the failure cases too. I see. I realized that I was debugging release Nightly which had e10s on, so I wasn't hitting my server breakpoints. Not a protocol problem. I'll keep digging.
Now that I'm running release Nightly with e10s off, I can't replicate the problem any more. I'll turn e10s back on and see if I can hit a server breakpoint to confirm the hideBoxModel message is being received.
I've confirmed that unrolling the working part of Bug 1453668 resolves this issue. Needinfo-ing Matt on this question: Matt, is the change made in Bug 1453668 to ProcessFrame (now ProcessFrameInternal) to constrain to currentFrame->IsFixedPosContainingBlock() somehow over-constraining? In this case, an SVG element is being given the "hidden" attribute, and that is getting properly translated into applying display:none to the element, but the frame region is not being included in the ProcessFrameInternal collection of dirty rects when we need it to be included. I'm having trouble breakpointing the ProcessFrameInternal method at the point where this frame is being considered, but I assume that the code is going through nsStyleDisplay::IsFixedPosContainingBlock and hitting https://searchfox.org/mozilla-central/source/layout/style/nsStyleStructInlines.h#190. If that's the case, then it's not allowing through any frames where nsSVGUtils::IsInSVGTextSubtree() returns true. I'm not sure what the rationale is for that exclusion. One possible fix for this problem would be for me to make a parameterized version of nsStyleDisplay::IsFixedPosContainingBlock that ignores this SVG check. I'll try that shortly. All insights appreciated.
Flags: needinfo?(matt.woodrow)
Thanks for looking into this Brad! Normally for retained display lists, whenever a frame changes, we rebuild the display items for that frame, as long as all items infront and behind it. However, when we have nested lists (like a transform having a sub-list of children), we only really need to build items infront/behind the modified frame within the list the modified frame is in. ProcessFrame iterates up the frame tree looking for a frame that would create a nested list, and if it finds one, then sets things up so we only build the modified area for that subtree of the frame tree. The change in bug 1453668 was changing the condition we use to identify these container frames. Previously we required them to be a stacking context (like transform/opacity), but now we also require them to be a containing block for fixed:position (which including being a containing block for everything). I am unsure why this affect SVG text. Display:none usually results in us deleting the nsIFrame*, and calling nsIFrame::RemoveDisplayItemDataForDeletion(). That should clear out any dangling references to the nsIFrame* on the display items created for that frame, and result in nsDisplayItem::HasDeletedFrame() returning true. The next paint won't have the frame in the set we iterate through in ProcessFrame (since it's deleted), and we won't add any area for it to the rebuild area. If this was the only change, then we expect to have no frame, and compute an empty area. After that, we should call RetainedDisplayListBuilder::MergeDisplayLists, which should call into [1] (either from ProcessItemFromNewList, or Finalize), which is where we decide *not* to include the display items from the deleted frame into the new list. I also noticed [2], which is called from ProcessFrame and might prove relevant if the above turns out not to be the situation here. [1] https://searchfox.org/mozilla-central/source/layout/painting/RetainedDisplayListBuilder.cpp#364 [2] https://searchfox.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#2952
Flags: needinfo?(matt.woodrow)
(In reply to Matt Woodrow (:mattwoodrow) from comment #18) > Display:none usually results in us deleting the nsIFrame*, and calling > nsIFrame::RemoveDisplayItemDataForDeletion(). That should clear out any > dangling references to the nsIFrame* on the display items created for that > frame, and result in nsDisplayItem::HasDeletedFrame() returning true. > > The next paint won't have the frame in the set we iterate through in > ProcessFrame (since it's deleted), and we won't add any area for it to the > rebuild area. If this was the only change, then we expect to have no frame, > and compute an empty area. Thanks so much for this info. I've confirmed that nsIFrame::RemoveDisplayItemDataForDeletion() is being called for the affected frame in PresShell::NotifyDestroyingFrame(), but then noticed the code hitting this line later in that function: https://searchfox.org/mozilla-central/source/layout/base/PresShell.cpp#2160. This surprised me. If a frame is destroyed, then we remove it from the mFramesToDirty list? That makes sense because the object is dying, but it doesn't make sense in that we still care about the region occupied by that frame. To test this, I manipulated my debugger to skip over the mFramesToDirty.RemoveEntry(aFrame) line for all the SVG frames affected by the display:none, and that resulted in correct behavior! So, how does the retained display list deal with a destroyed frame whose area needs to be dirtied and repainted? This seems like a common case that I presume is handled somehow. Is there something about this SVG frame that prevents it from being correctly handled?
Flags: needinfo?(matt.woodrow)
Interesting! mFramesToDirty is the set of frames that we want to run Reflow for, and isn't directly related to display lists at all. I would guess that running reflow on them again results in them being put back into the set of frames that we use for ProcessFrame. Note that doing this will result in the frame being accessed after it's deleted, so the contents on the nsIFrame* will likely be the poison value, or possibly belong to a new frame allocated into the same memory. For dirtying and repainting, there's two things that need to happen: * We need to remove the display items that no longer have frames from the display list. RemoveDisplayItemDataForDeletion() should disown all display items for the deleted frame, and MergeDisplayLists should handle detecting those and omitting them. * We need to notify the code that manages the retained pixel buffers (FrameLayerBuilder) to tell it to dirty pixels covered by the deleted frame. We do this synchronously during deletion, to prevent us getting confused if the frame memory gets reused (common with the arena). RemoveDisplayItemDataForDeletion calls FrameLayerBuilder::RemoveFrameFromLayerManager, which iterates its internal state about what that frame painted, and dirties pixels covered by it. Enabling the PrintDisplayList calls in RetainedDisplayListBuilder::AttemptPartialUpdate can be useful to verify the display list building part. I'd lean towards that (given the regression range), but can't rule out downstream effects.
Flags: needinfo?(matt.woodrow)
Brad, do you have what you need to move forward with this now?
Flags: needinfo?(bwerth)
Does this still reproduce for anyone? I can't reproduce it, and I now suspect that this is a duplicate of bug 1461231.
It does not reproduce anymore. Judging by the work done in bug 1461231, which is solving the problem noted in comment 19, this is certainly a duplicate. And that bug's patch has been uplifted. Wonderful!
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(bwerth)
Resolution: --- → DUPLICATE
Great news! Thanks everyone!
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: