Closed Bug 1021564 Opened 5 years ago Closed 5 years ago

[toolbox] Toggling toolbox buttons acts weirdly

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla34
Tracking Status
firefox32 --- verified
firefox33 + verified
firefox34 --- verified

People

(Reporter: pbro, Assigned: mstange)

References

Details

(Keywords: regression)

Attachments

(7 files, 1 obsolete file)

Not sure how to phrase this bug's summary better, but this video explains it all:

https://www.youtube.com/watch?v=0kTRuEMcNPs

While disabling toolbox buttons, at some stage, 2 weird things happens:
- disabling the responsive mode button removes the button but inserts a new paintflashing button
- going on and removing more buttons: the paintflashing button stays visible in the middle of the toolbar and only goes away when the window it is resized.

I managed to reproduce this problem locally on nightly.
The order in which you disable buttons seems to be important.
Have them all selected, and then start unselecting from the top (pick an element, split console, responsive mode, ...).
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #0)
> Not sure how to phrase this bug's summary better, but this video explains it
> all:
> 
> https://www.youtube.com/watch?v=0kTRuEMcNPs
> 
> While disabling toolbox buttons, at some stage, 2 weird things happens:
> - disabling the responsive mode button removes the button but inserts a new
> paintflashing button
> - going on and removing more buttons: the paintflashing button stays visible
> in the middle of the toolbar and only goes away when the window it is
> resized.
> 
> I managed to reproduce this problem locally on nightly.
> The order in which you disable buttons seems to be important.
> Have them all selected, and then start unselecting from the top (pick an
> element, split console, responsive mode, ...).

I noticed this when we started having the display glitches in Nightly.  The weird thing is that if you resize the toolbox at any stage things seems to get back in sync.  I can check with browser toolbox to confirm that buttons are actually being removed
Randomly removing/adding buttons, I managed to get to this stage:
https://dl.dropboxusercontent.com/u/714210/Screenshot%202014-06-06%2014.45.25.png

Also worth noting that the problem goes away as soon as you switch to another app too, no need to resize the window.
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #2)
> Randomly removing/adding buttons, I managed to get to this stage:
> https://dl.dropboxusercontent.com/u/714210/Screenshot%202014-06-06%2014.45.
> 25.png
> 
> Also worth noting that the problem goes away as soon as you switch to
> another app too, no need to resize the window.

I can see that too, but only if the toolbox is at a certain resolution.  If I make the window more narrow the problem seems to go away.
Does it still happen if you switch layers.offmainthreadcomposition.enabled to false?
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #4)
> Does it still happen if you switch layers.offmainthreadcomposition.enabled
> to false?

Yes, still happens after setting that pref and restarting
Good: cbf99fb64ee9 (1399923357 inbound)
Bad: 06800eda477b (1399923539 inbound)
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cbf99fb64ee9&tochange=06800eda477b

So this is caused by Bug 1008301 or Bug 1008578.
I can reproduce it on Windows XP, so the Operating System field should be changed to All (I don't have enough privileges to change it).
OS: Mac OS X → All
Seems like only the light theme is affected, couldn't reproduce on dark theme.
Maybe it's related to the fact that we use CSS Filters.
(In reply to Tim Nguyen [:ntim] from comment #8)
> Seems like only the light theme is affected, couldn't reproduce on dark
> theme.
> Maybe it's related to the fact that we use CSS Filters.

Weird, it only seems to be an issue on the light theme for me too.
I think that removing command buttons from the big list of filter elements does actually remove the glitch: https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/toolbars.inc.css#783.  Of course, you can't see any of the images anymore because they are white, but this helps narrow it down.
Attached patch hack-translate.patch (obsolete) — Splinter Review
This actually fixes the problem for me, but I don't know about landing it this way
(In reply to Brian Grinstead [:bgrins] from comment #11)
> Created attachment 8437971 [details] [diff] [review]
> hack-translate.patch
> 
> This actually fixes the problem for me, but I don't know about landing it
> this way

I think there's a will-change property implemented in Firefox (bug 940842). It's an alternative to the transform hack. Although, I've never tested it.
Attached file filters-bug.zip
I see the same issue in web content when pressing the 'hide1' button here
(In reply to Brian Grinstead [:bgrins] from comment #13)
> Created attachment 8437989 [details]
> filters-bug.zip
> 
> I see the same issue in web content when pressing the 'hide1' button here

Markus, does this issue seem like it could have been caused by Bug 1008301 or Bug 1008578?  These were the bugs tracked down in Comment 6.
Flags: needinfo?(mstange)
Definitely.
Assignee: nobody → mstange
Blocks: 1008301
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
Keywords: regression
Component: Developer Tools → Layout
Product: Firefox → Core
Version: unspecified → Trunk
Attached file testcase 1
Hovering the document should make the 100x100 green square move and not leave any green stuff behind.
Attached file testcase 2
should behave the same as testcase 1
This is affecting 32. The problem is seen on https://bug1021564.bugzilla.mozilla.org/attachment.cgi?id=8439797 in the latest Aurora 32.0a2 (2014-07-15)
Sorry, I should have added a comment with that change.
This is fixed on Aurora since yesterday because I backed out bug 1008301 from Aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/0fcb1dc73d58
Attached patch v1Splinter Review
Attachment #8456866 - Flags: review?(roc)
Comment on attachment 8456866 [details] [diff] [review]
v1

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

::: layout/base/nsDisplayListInvalidation.h
@@ +135,5 @@
> +class nsDisplaySVGEffectsGeometry : public nsDisplayItemGeometry
> +{
> +public:
> +  nsDisplaySVGEffectsGeometry(nsDisplaySVGEffects* aItem, nsDisplayListBuilder* aBuilder);
> +  

trailing whitespace
Attachment #8456866 - Flags: review?(roc) → review+
Attachment #8437971 - Attachment is obsolete: true
sorry had to backout this change for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=44026850&tree=Mozilla-Inbound&full=1 

06:49:04     INFO -  [965] ###!!! ASSERTION: How did we getting here, then?: 'aFrame->GetParent()->StyleContext()->GetPseudo() == nsCSSAnonBoxes::mozAnonymousBlock', file /builds/slave/m-in-osx64-d-00000000000000000/build/layout/svg/nsSVGIntegrationUtils.cpp, line 111
06:49:04     INFO -  PreEffectsVisualOverflowCollector::AddBox(nsIFrame*) [layout/svg/nsSVGIntegrationUtils.cpp:62]
06:49:04     INFO -  nsLayoutUtils::GetAllInFlowBoxes(nsIFrame*, nsLayoutUtils::BoxCallback*) [layout/base/nsLayoutUtils.cpp:3413]
06:49:04     INFO -  GetPreEffectsVisualOverflowUnion [obj-firefox/dist/include/nsRect.h:39]
06:49:04     INFO -  nsSVGIntegrationUtils::GetSVGBBoxForNonSVGFrame(nsIFrame*) [obj-firefox/dist/include/nsRect.h:54]
06:49:04     INFO -  nsSVGUtils::GetBBox(nsIFrame*, unsigned int) [layout/svg/nsSVGUtils.cpp:984]
06:49:04     INFO -  nsDisplaySVGEffectsGeometry::nsDisplaySVGEffectsGeometry(nsDisplaySVGEffects*, nsDisplayListBuilder*) [layout/base/nsDisplayList.cpp:5262]
06:49:04     INFO -  nsDisplaySVGEffects::AllocateGeometry(nsDisplayListBuilder*) [layout/base/nsDisplayList.h:3141]
06:49:04     INFO -  mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*, unsigned int) [layout/base/FrameLayerBuilder.cpp:2922]
06:49:04     INFO -  mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, gfx3DMatrix const*, unsigned int) [layout/base/FrameLayerBuilder.cpp:603]
06:49:04     INFO -  nsDisplayTransform::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) [obj-firefox/dist/include/nsCOMPtr.h:175]
06:49:04     INFO -  mozilla::FrameLayerBuilder::AddThebesDisplayItem(mozilla::ThebesLayerData*, nsDisplayItem*, mozilla::DisplayItemClip const&, nsIntRect const&, mozilla::ContainerState const&, mozilla::LayerState, nsPoint const&, nsAutoPtr<nsDisplayItemGeometry>) [obj-firefox/dist/include/nsCOMPtr.h:175]
06:49:04     INFO -  mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*, unsigned int) [obj-firefox/dist/include/nsAutoPtr.h:73]
06:49:04     INFO -  mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, gfx3DMatrix const*, unsigned int) [layout/base/FrameLayerBuilder.cpp:603]
06:49:04     INFO -  nsDisplayOwnLayer::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) [obj-firefox/dist/include/nsCOMPtr.h:175]
06:49:04     INFO -  nsDisplaySubDocument::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) [obj-firefox/dist/include/nsCOMPtr.h:175]
06:49:04     INFO -  mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*, unsigned int) [obj-firefox/dist/include/nsCOMPtr.h:175]
06:49:04     INFO -  mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, gfx3DMatrix const*, unsigned int) [layout/base/FrameLayerBuilder.cpp:603]
06:49:04     INFO -  nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) [layout/base/nsIPresShell.h:1328]
06:49:04     INFO -  nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) [obj-firefox/dist/include/GeckoProfilerImpl.h:370]
06:49:04     INFO -  PresShell::Paint(nsView*, nsRegion const&, unsigned int) [layout/base/nsPresShell.cpp:6222]
06:49:04     INFO -  nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) [obj-firefox/dist/include/nsRegion.h:55]
06:49:04     INFO -  nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) [view/src/nsViewManager.cpp:380]
06:49:04     INFO -  nsRefreshDriver::Tick(long long, mozilla::TimeStamp) [layout/base/nsLayoutUtils.h:2067]
06:49:04     INFO -  mozilla::RefreshDriverTimer::Tick() [obj-firefox/dist/include/nsTArray.h:855]
06:49:04     INFO -  nsTimerImpl::Fire() [xpcom/threads/nsTimerImpl.cpp:634]
06:49:04     INFO -  nsTimerEvent::Run() [obj-firefox/dist/include/nsAutoPtr.h:836]
06:49:04     INFO -  nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:766]
06:49:04     INFO -  NS_ProcessPendingEvents(nsIThread*, unsigned int) [xpcom/glue/nsThreadUtils.cpp:207]
06:49:04     INFO -  nsBaseAppShell::NativeEventCallback() [widget/xpwidgets/nsBaseAppShell.cpp:99]
06:49:04     INFO -  nsAppShell::ProcessGeckoEvents(void*) [widget/cocoa/nsAppShell.mm:375]
06:49:04     INFO -  CoreFoundation + 0x4e401
06:49:04     INFO -  CoreFoundation + 0x4c5f9
06:49:04     INFO -  CoreFoundation + 0x4bdbf
06:49:04     INFO -  HIToolbox + 0x2e7ee
06:49:04     INFO -  HIToolbox + 0x2e5f3
06:49:04     INFO -  HIToolbox + 0x2e4ac
06:49:04     INFO -  AppKit + 0x43eb2
06:49:04     INFO -  -AppKit + 0x43801
06:49:04     INFO -  -[G
Duplicate of this bug: 1046228
I backed out bug 1008301 (which caused this) from Aurora again so that this bug is fixed on 33.

[Tracking Requested - why for this release]: Requesting tracking to be moved from 33 to 34.

Hopefully I'll be able to finish this up today or tomorrow.
Blocks: 1057180
A testcase with CSS Filters. The bug is reproducible too.
This is how attachment 8477372 [details] should look like.
Duplicate of this bug: 1059493
Attached patch mathml fixSplinter Review
The testcase that the assertion from comment 23 fires on (473278-1.xhtml) looks like this:

<html xmlns="http://www.w3.org/1999/xhtml"><head></head><body><mmultiscripts xmlns="http://www.w3.org/1998/Math/MathML" style="clip-path: url(#q); -moz-transform: translate(100px, 100px);"/></body></html>

and has this frame tree:

> Viewport(-1)@12945d450 [view=12937df20] {0,0,48000,60000} [state=0001062000002220] [sc=129a2dea8:-moz-viewport]<
>   HTMLScroll(html)(-1)@12945e190 {0,0,48000,60000} [state=0000020000000010] [content=129458480] [sc=129a2e9d0:-moz-viewport-scroll]<
>     ScrollbarFrame(scrollbar)(-1)@12945e978 next=1294ebea0 {0,60000,48000,0} [state=0000100080c80000] [content=129428a10] [sc=129a2ef50]<
>       SliderFrame(slider)(-1)@1294eb520 {0,0,48000,960} [state=0000160080c00000] [content=129428d70] [sc=12945edf8]<
>         ButtonBoxFrame(thumb)(0)@1294eb9e8 {60,120,47880,780} [state=2000160080400000] [content=129428e00] [sc=1294ebfa0]<>
>       >
>     >
>     ScrollbarFrame(scrollbar)(-1)@1294ebea0 next=129a2d2b0 {48000,0,0,60000} [state=0000100080880000] [content=129428aa0] [sc=1294eb750]<
>       SliderFrame(slider)(-1)@1294ec7f8 {0,0,960,60000} [state=0000160080800000] [content=129429280] [sc=1294ec280]<
>         ButtonBoxFrame(thumb)(0)@1294ecc30 {120,60,780,59880} [state=2000160080000000] [content=129429310] [sc=129a2ddf0]<>
>       >
>     >
>     Box(scrollcorner)(-1)@129a2d2b0 next=12945de80 {48000,60000,0,0} [state=0000100080c00200] [content=129428b30] [sc=1294ec998]<>
>     Canvas(html)(-1)@12945de80 {0,0,48000,60000} [state=0000002000000200] [content=129458480] [sc=1294ece28:-moz-scrolled-canvas]<
>       Block(html)(-1)@129a2d910 {0,0,48000,2112} [state=0000100000d00200] [content=129458480] [sc=129a2d470,parent=0]<
>         line 129a2dfe8: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x48] bm=480 {480,480,47040,1152} <
>           Block(body)(1)@129a2df50 {480,480,47040,1152} [state=0000120000100200] [content=129485520] [sc=129a2dac0]<
>             line 129a2e328: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x100] {0,0,5914,1152} <
>               Frame(mmultiscripts)(0)@129a2e260 {0,96,5914,960} [state=0000100000010000] transformed [content=1294586c0] [sc=12945d0f8]<>
>             >
>           >
>         >
>       >
>     >
>   >
> >

The mmultiscripts frame is in error and shows "invalid-markup".

We call nsSVGUtils::GetBBox on the mmultiscripts frame and assert because that frame doesn't have a PreEffectsBBoxProperty. The PreEffectsBBoxProperty would be set in ComputeEffectsRect during FinishAndStoreOverflow, but the mmultiscripts frame never calls FinishAndStoreOverflow because nsMathMLContainerFrame::FinalizeReflow, which would usually call FinishAndStoreOverflow through GatherAndStoreOverflow, returns early due to the error state.

Can we just call GatherAndStoreOverflow here regardless? Or is there a better fix?

https://tbpl.mozilla.org/?tree=Try&rev=fb7ea1e10c8a
Attachment #8481781 - Flags: review?(roc)
Comment on attachment 8481781 [details] [diff] [review]
mathml fix

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

looks good to me
Attachment #8481781 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/9c0d8ef32b3e
https://hg.mozilla.org/mozilla-central/rev/2c0f68531d4b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa+]
Reproduced issue with steps from comment 0 and also testcases from comment 16 and comment 17, using Nightly from August 29 (BuildID=20140829030204), on Windows 7 x64, Mac OS X 10.9.4, Ubuntu 13.04 x64.

No issues were seen on Windows 7 x64, Mac OS X 10.9.4, Ubuntu 13.04 x64, using:
- Firefox 32 Release Candidate - BuildID=20140825202822
- Latest Firefox 33 Aurora - BuildID=20140831004002
- Latest Firefox 34 Nightly - BuildID=20140831030206
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.