If sending a notification locks up the whole shell for several seconds, that's a shell problem.

Are you sending us a giant image file?

Rhythmbox sends images using the image_path hint rather than shoving a pixbuf through dbus, but the images could be any size.

It would be odd that it would be due to album cover art, as 1) I didn't experience that issue before, and my collection hasn't changed in recent times 2) it happens regardless of the song/album.

FWIW, I tried reloading the shell or rebooting, but the problem persists (hence the bug report).

Jasper, FWIW this seems fixed with the "3.10.4-5.fc20" version that trickled down to Fedora 20's stable updates today. It seems like was previously running 3.10.4-4.fc20.

I don't know why this solves the issue, but maybe you can find out from that link: https://admin.fedoraproject.org/updates/gnome-shell-3.10.4-5.fc20

Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 730118 ***

Florian, I don't quite understand how the notifications are linked to gtk 3.12 + ATK? (not even in use on this machine, I'm using the 3.10 stack) are you sure about this being a dupe of 730118?

Pretty sure, yeah. There was a request to backport https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 in Fedora after 3.10.4-2, which has the side effect of always ending up initializing ATK; luckily testers caught the serious performance regressions (incidentally as well triggered by Rhythmbox) and the 3.10.4-3 update was never pushed.

However the bad patch made it back accidentally in 3.10.4-4 and was not stopped in testing (mostly because it was pushed with a low karma threshold). Now 3.10.4-5 did nothing apart from removing the patch again - so if that update indeed fixes your problem, that's a fairly strong hint ...

(In reply to comment #8)
> Pretty sure, yeah. There was a request to backport
> https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 in Fedora after
> 3.10.4-2, which has the side effect of always ending up initializing ATK;

Do you mean atspi not atspi? 

By the way regarding https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 initialisation of atspi would only happen under the circumstance that the magnifier was activated *and* either caret or focus tracking was registered with that patch but that did not pass review which is why you would have seen the problem see: https://bugzilla.gnome.org/show_bug.cgi?id=723709 for the original patch.

> luckily testers caught the serious performance regressions (incidentally as
> well triggered by Rhythmbox) and the 3.10.4-3 update was never pushed.

which update are you referring to?
> 
> However the bad patch made it back accidentally in 3.10.4-4 and was not stopped
> in testing (mostly because it was pushed with a low karma threshold). Now
> 3.10.4-5 did nothing apart from removing the patch again - so if that update
> indeed fixes your problem, that's a fairly strong hint ...

(In reply to comment #9)

> Do you mean atspi not atspi? 

or rather atspi not atk, even!

In case that was confusing you can just apply this to test wether atspi is your problem in for this bug

https://bug723709.bugzilla-attachments.gnome.org/attachment.cgi?id=268210

Here I assume your focusCaretTracker.js looks like this though so be careful:

https://git.gnome.org/browse/gnome-shell/commit/js/ui/focusCaretTracker.js?id=7ced1f5b543dc2ea6bd52230bdec1c0e12959740

(In reply to comment #9)
> (In reply to comment #8)
> > Pretty sure, yeah. There was a request to backport
> > https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 in Fedora after
> > 3.10.4-2, which has the side effect of always ending up initializing ATK;
> 
> Do you mean atspi not atk? 

Yeah, sorry.


> By the way regarding
> https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 initialisation
> of atspi would only happen under the circumstance that the magnifier was
> activated *and* either caret or focus tracking was registered

No. It is initialized by the FocusCaretTracker, which is created by the ZoomRegion. Before that commit, the ZoomRegion was created in Magnifier._initialize() the first time the magnifier was activated. After that commit this happens in Magnifier._init(), e.g. always (because the magnifier is created unconditionally in main.js).


> > luckily testers caught the serious performance regressions (incidentally as
> > well triggered by Rhythmbox) and the 3.10.4-3 update was never pushed.
> 
> which update are you referring to?

This one: http://pkgs.fedoraproject.org/cgit/gnome-shell.git/commit/?h=f20&id=e67534d3e0 (it was not pushed because of the problems, but I didn't revert it in git either, so it was pushed out with the update in the next commit).


> > However the bad patch made it back accidentally [...]

I guess I should clarify that by "bad patch" I meant: the patch that triggered the performance regressions - the real problem is that initializing atspi has a big performance impact like that. Having the caret tracker always on (or at least activated by default) would make a lot of sense for instance for making sure that notifications don't pop up right over the entry the user is currently writing in (which has been in the mockups for ages). So I'm sorry if I sounded like criticizing your work - it exposes a problem in atspi, it is not the problem itself (unfortunately this means that it shouldn't be dumped on users of a stable distro, but that blame is mostly on me)

(In reply to comment #12)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Pretty sure, yeah. There was a request to backport
> > > https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 in Fedora after
> > > 3.10.4-2, which has the side effect of always ending up initializing ATK;
> > 
> > Do you mean atspi not atk? 
> 
> Yeah, sorry.
> 
> 
> > By the way regarding
> > https://git.gnome.org/browse/gnome-shell/commit?id=8d8d1cfdd620 initialisation
> > of atspi would only happen under the circumstance that the magnifier was
> > activated *and* either caret or focus tracking was registered
> 
> No. 

Yes. These methods would have to have been called with focus or caret tracker setting mode != none before ATSPI would be inited.

      * setFocusTrackingMode
+     * method which sets gsetting mode for focus-tracking
+     *  then it registers focus-changed events and will allow
+     * Atspi.init to be called in focusCaretTracker.js only
+     * if focus-tracking mode parameter is not none.
      * @mode:     One of the enum FocusTrackingMode values.
      */
     setFocusTrackingMode: function(mode) {
         this._focusTrackingMode = mode;
-        if (this._focusTrackingMode == GDesktopEnums.MagnifierFocusTrackingMode.NONE)
+        if (this._focusTrackingMode == GDesktopEnums.MagnifierFocusTrackingMode.NONE) {
             this._focusCaretTracker.deregisterFocusListener();
-        else
+        } else {
+            this._focusCaretTracker.connect('focus-changed',
+                                    Lang.bind(this, this._updateFocus));
             this._focusCaretTracker.registerFocusListener();
+        }
     },
 
     /**
      * setCaretTrackingMode
-     * @mode:     One of the enum CaretTrackingMode values.
+     * @mode:     One of the enumvalues.
+     * method which sets gsetting mode for caret-tracking
+     * then it registers caret-moved events and will allow
+     * Atspi.init to be called in focusCaretTracker.js only
+     * if caret-tracking mode parameter is not none.
+     * @mode:     One of the enum  values.
      */
     setCaretTrackingMode: function(mode) {
         this._caretTrackingMode = mode;
-        if (this._caretTrackingMode == GDesktopEnums.MagnifierCaretTrackingMode.NONE)
+        if (this._caretTrackingMode == GDesktopEnums.MagnifierCaretTrackingMode.NONE) {
             this._focusCaretTracker.deregisterCaretListener();
-        else
+        } else {
+            this._focusCaretTracker.connect('caret-moved',
+                                    Lang.bind(this, this._updateCaret));
             this._focusCaretTracker.registerCaretListener();
+        }
     },

That was the design used in the initial patch I submitted that was rejected.

(In reply to comment #12)
> (In reply to comment #9)
> > (In reply to comment #8)

> > > However the bad patch made it back accidentally [...]
> 
> I guess I should clarify that by "bad patch" I meant: the patch that triggered
> the performance regressions - the real problem is that initializing atspi has a
> big performance impact like that. Having the caret tracker always on (or at
> least activated by default) would make a lot of sense for instance for making
> sure that notifications don't pop up right over the entry the user is currently
> I'm sorry if I sounded like criticizing your work - it exposes a problem in 
> atspi, it is not the problem itself (unfortunately this means that it ?>shouldn't be dumped on users of a stable distro, but that blame is mostly on me)

Don't worry I am not precious about my work being critiqued, I prefer honesty, actually. That is constructive. Review and testing for bugs is part of the process accordingly, I do not really understand how bugs could be filed if nobody is willing to say when a bit of code is broken for fear of offending someone else! That probably, no: certainly, would not be a very productive way to solve problems so you can always feel free to point out facts as far as my patches are concerned. ;-) Ultimately, everyone makes mistakes, it is how we deal with them that is what sorts art from fart :-)

That said, the decision to not include above methods was not mine since my initial patch handled it the above methods but I had to take them out to get my code in, which is fine. I appreciate the rationale for that. I expect you all want to have a clear idea of what a patch is doing and make a patch per change but since the methods were written and it would have seemed at the time that this issue was separate to crosshairs. In fact, when you think about it, the issue is separate. The decision was right.

Still though, had the methods above been included in the first place then the only issue to deal with would be un-initing in atspi, from what Mike and Alejandro seem to be saying, anyway. So, you would think someone would have thought to mentioned this issue at the time it was noticed first, since the code which dealt with initing was already written... Just sayin'

To be honest though there is an important point that I worry is being overlooked in all this: Whilst atspi is needed in gnome shell for disabled users it is important to try to implement it in such a way that minimises the risk that someone could inject code via an extension or browser that can exploit it's functions to spy on a user (disabled or otherwise).

For example, it is very easy to make a keylogger using atspi. Very easy to upload it as an extension too, no doubt. That seems like the sort of thing to try and protect all users from whether they are disabled or not so it might be worth considering whether initing atspi in javascript  (or even register listeners for it in js, for that matter) should be an option in gnome shell. I cannot say what the best prevention strategy is but I suspect addressing the potential risks might be a good place to start.

(In reply to comment #13)
> Yes. These methods would have to have been called with focus or caret tracker
> setting mode != none before ATSPI would be inited.

No, this is about Atspi.init(), not (de)registering listeners, see https://bugzilla.gnome.org/show_bug.cgi?id=730118#c28.

(In reply to comment #14)
> (In reply to comment #13)
> > Yes. These methods would have to have been called with focus or caret tracker
> > setting mode != none before ATSPI would be inited.
> 
> No, this is about Atspi.init(), not (de)registering listeners, see
> https://bugzilla.gnome.org/show_bug.cgi?id=730118#c28.

As I said 

(In reply to comment #14)
> (In reply to comment #13) 
> Still though, had the methods above been included in the first place then the
> only issue to deal with would be un-initing in atspi, from what Mike and
> Alejandro seem to be saying, anyway.