After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769705 - Can't navigate to next or previous change if file not writeable
Can't navigate to next or previous change if file not writeable
Status: RESOLVED FIXED
Product: meld
Classification: Other
Component: filediff
3.16.x
Other Linux
: Normal normal
: ---
Assigned To: meld-maint
meld-maint
: 769709 771878 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-08-10 14:38 UTC by Ed Adasiewicz
Modified: 2016-10-02 14:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
first file for comparison (20.21 KB, text/plain)
2016-08-17 13:19 UTC, Ed Adasiewicz
  Details
second file for comparison (20.22 KB, text/plain)
2016-08-17 13:19 UTC, Ed Adasiewicz
  Details
Workaround (2.35 KB, patch)
2016-08-21 01:40 UTC, Kai Willadsen
rejected Details | Review
Workaround2 - coerce value to integer inside meld (1.07 KB, patch)
2016-09-13 16:50 UTC, Vasily Galkin
none Details | Review
Workaround2 - coerce value to integer inside meld (1.07 KB, patch)
2016-09-13 16:54 UTC, Vasily Galkin
none Details | Review

Description Ed Adasiewicz 2016-08-10 14:38:22 UTC
I actually stumbled upon this while using meld with subversion.  If I used "svn diff" without specifying a revision then I was unable to navigate between differences (although they were displayed) but if I used a revision number then everything worked fine.  Looking further I noticed that with no revision specified that subversion was pulling the file locally from "pristine" and it had a mode of 0444 whereas the one with a revision had a mode of 0644.  I then proceeded to extract 2 revisions from the subversion repository and compare the 2 with meld.  If the mode bits were the same on both then all worked as expected but if I turned off the write mode on one of the files then I could not navigate (either keyboard or toolbar) between differences.  Also the terminal screen had the following error message repeated many times:

(meld:20949): Gtk-WARNING **: Allocating size to GtkVBox 0x559547427120 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?

I'm not sure if this is a meld problem or a gnome problem or a configuration problem.  Thanks.
Comment 1 Kai Willadsen 2016-08-16 21:31:26 UTC
I've tested this by just making a random file not writable, and it seems to work fine for me. If there's any way you could attach files to reproduce, that would help a lot.

Just to confirm, when you reproduced this by extracting files from the repo, you were just comparing two files in a normal two-pane comparison, not anything else?

Do the actual next/prev diff actions display as sensitive when this is happening?



That warning, while annoying, should be unrelated and is because of GTK+ 3.20.
Comment 2 Ed Adasiewicz 2016-08-17 13:19:22 UTC
Created attachment 333495 [details]
first file for comparison
Comment 3 Ed Adasiewicz 2016-08-17 13:19:55 UTC
Created attachment 333497 [details]
second file for comparison
Comment 4 Ed Adasiewicz 2016-08-17 13:28:34 UTC
I attached 2 files for comparison.  They only differ by 3 lines.  If both files have their file mode bits set to 0644 then everything is OK.  If I change the .02 file to have mode bits of 0444 then the problem occurs.  A couple of items I noticed were:

1.  The error message only occurs when the comparison is not working correctly, but the line number changed from 20949 to 22891.

2.  If I watch the "status line" in the meld application I can see where the value of Ln changes but the highlight line is not moving -- perhaps this is just a gtk problem.

3.  It did not make any difference if the file with mode bits 0444 was first or second on the command line.

I am running Arch Linux with all of the latest updates but nothing from testing/beta.  The version of meld is 3.16.2-1.  Let me know if you need anything else.
Comment 5 Kai Willadsen 2016-08-21 01:37:12 UTC
Thanks for the information. This is a new issue that is (best I can tell) a regression in GTK+ 3.20. I've linked the new GTK+ bug, since I can't find an easy workaround that doesn't do something else bad.
Comment 6 Kai Willadsen 2016-08-21 01:40:25 UTC
Created attachment 333781 [details] [review]
Workaround

This works around the issue in my testing. I'm not really willing to apply it, since it's not how the UI is supposed to work. Another option would be to disable this toggle button entirely for GTK+ 3.20 and later... which I also don't really want to do for obvious reasons.
Comment 7 Kai Willadsen 2016-08-21 03:14:31 UTC
*** Bug 769709 has been marked as a duplicate of this bug. ***
Comment 8 Vasily Galkin 2016-08-29 17:34:25 UTC
there is a chance that the problem exits only with GTK+ 3.20.

I failed to reproduce this isuue in my environment but have several problems that looks related to this.
Most of them are rare and randomly-reproducible, but there is one that is quite stable - "do not scroll to search result", so here is its description.

1. Open unchanged file compare via meld's vcview mechanism. Vertically it should occupy at least 1.5 screen heights, so it can be vertically scrolled by a huge amount. For example use "README" in meld's git. (exec bin/meld README)

2. Press "Hide" in "files are identical" message. This step is important.

2. Start searching via pressing "Find" in menu.

3. Enter a substring that matches several times, both at the top and at the bottom of the screen. "li" for meld's README file.

4. Press the "<Previous" button near search field several times.

Actual result: sometimes currently highlighted (blue selection color) match is not scrolled into view. Line position and "Wrapped" indicator does always change correctly.

Expected result: currently highlighted match is always scrolled into view.

Now about versions.
I have seen "no scroll to search result" problem on
windows,32-bit python.exe,pygobject-win32 3.18.2
windows,64-bit python.exe,pygobject-win32 3.18.2
debian,64-bit gtk&gtksourceview 3.18.x

And, definitely strange, I never see any scrolling problems with
debian,32-bit gtk&gtksourceview 3.20.x
while it produces "without calling gtk_widget_get_preferred_width/height()." warnings for GtkToggleToolButton and GtkVBox.

So a problem "do not scroll to search result" while being very similar to what is described here in https://bugzilla.gnome.org/show_bug.cgi?id=769705#c4 appears in some environments with gtk 3.18 where there is no warnings visible in console.

And also doesn't appear in some environments with gtk 3.20 with warnings visible.

While the versions are different I still think that the problems are related:
1)They are very similar
2)After applying attached workaround patch on windows and 64bit python.exe, the "no scroll to search result" disappears in some scenarious (but not in the one described above). It may be reasoned by the fact that the gtk 3.20 warning is raised not only with GtkToggleToolButton but with GtkVBox too, during plain scrolling.

Also, here is list of other scrolling problems that I seen several times in  environments where the "do not scroll to search result" problem appears:
-scroll cursor out-of-view after typing in new letter
-scroll cursor out-of-view when crossing line boundaries with left/right arrows
-doesn't scroll to first difference after file loading (very rare, exactly like https://bugzilla.gnome.org/show_bug.cgi?id=759762 ).

Unfortunately exact steps to reproduce those problems were not found.
Comment 9 Vasily Galkin 2016-08-29 17:35:37 UTC
Mistype, I mean that the problem exists *NOT* only with GTK+ 3.20.
Comment 10 Daniel D 2016-09-08 16:52:35 UTC
The problem does not only occur on read-only files, but also when I do a blank comparison, which should happen in RAM or at least on writeable temp files.
In short: the problem occurs for all files that are longer than the screen height and require scrolling.

I get the following error several times, I try to use Alt+Up/Down to navigate to the next change, or when I scroll in the meld window:
(meld:1929): Gtk-WARNING **: Allocating size to GtkVBox 0x55f53c13be40 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?

System:
Manjaro Linux x64 (somewhat Arch based)
Xfce 4.12
Kernel 4.7.2-1-MANJARO
Meld 3.16.2-1
gtk3 3.20.9-1

Let me know if you need more info, or if I can test anything.
Comment 11 Kai Willadsen 2016-09-10 23:54:26 UTC
(In reply to Vasily Galkin from comment #8)
> there is a chance that the problem exits only with GTK+ 3.20.
> 
> I failed to reproduce this isuue in my environment but have several problems
> that looks related to this.
> Most of them are rare and randomly-reproducible, but there is one that is
> quite stable - "do not scroll to search result", so here is its description.

Thanks for this test case. It's helped me narrow down at least one of the problems here; whether or not it's the only problem I don't know.

The workaround for the larger problem, I believe, is to disable animation. From where I'm standing, this essentially means that whenever GTK+ added animation to scrolling via GtkAdjustments, they broke ABI yet again but that's not really a surprise at this point.

The problem here is that Meld has multiple adjustments for its multiple comparison windows, and keeps them in sync. However, when the adjustments are animating, we get several value-changed events for movement to an end location. Because the intermediate value-changed events trigger an update of the other panes, we get into a situation where only the first value-changed event triggers, and we scroll part of the way.

Ideally, we'd just straight-up ignore any value-changed events that were sourced from an animation, but that information (as well as whether the adjustment is animating) isn't exposed by GtkAdjustment, *even though it's used by GtkTextView for basically the same issue*.

I'll see if I can find a workaround as soon as I stop being so immensely angry about this.
Comment 12 Vasily Galkin 2016-09-13 16:50:58 UTC
Created attachment 335454 [details] [review]
Workaround2 - coerce value to integer inside meld

I performed a rough investigation in my windows environment with looking to Gtk.Adjustment's code.

It looks that the root of the problem are not scrolling animation value-changed events themselves. They are *NOT* fired as a result of meld's adj.set_value call. They are fired only after private call that is used by gtk's widgets and is not exposed outside of gtk.

The reason is some coercion mechanics.
For example when meld calls adj.set_value with the 578.7142160000001 argument it synchronously receives value-changed event with same 578.7142160000001 argument (which is filtered out by meld's recursive call protection).

And then another asynchronous value-changed event with 578.0 value is received. Such asynchronous events always has zero decimal part, number is an integer. And those events are problem: unlike animation they are not reaction to user input, but are raised after meld's set_value.

I've not found the gtk's code that performs coercion yet.
But here is dirty workaround that resolves issue in my environment:
just coerce value to integer inside meld before passing it to gtk.
Attaching diff.
Comment 13 Vasily Galkin 2016-09-13 16:54:08 UTC
Created attachment 335456 [details] [review]
Workaround2 - coerce value to integer inside meld

(rebase patch on meld-3-16 instead of my code)
Comment 14 Catalin Francu 2016-09-14 13:44:49 UTC
Just writing to confirm the behavior and workaround. I am on an up-to-date installation of Arch Linux. It comes with Meld 3.16.2 and GTK 3.20.9. The problem also arises on the Git version of Meld (3.17.0, it says).

Vasily Galkin's workaround2 worked for me.

As a bit of extra information, when I press Alt+Down and Alt+Up to navigate between changes, the cursor moves correctly, it goes to the beginning of the line for the appropriate change. If I scroll using the mouse wheel I can see it. It's just that the window doesn't scroll automatically to bring the change into view.
Comment 15 Kai Willadsen 2016-09-16 22:36:07 UTC
(In reply to Vasily Galkin from comment #12)
> It looks that the root of the problem are not scrolling animation
> value-changed events themselves. They are *NOT* fired as a result of meld's
> adj.set_value call. They are fired only after private call that is used by
> gtk's widgets and is not exposed outside of gtk.

It's true that these are fired by internal GTK+ machinery, but we get all of the events.

> The reason is some coercion mechanics.
> For example when meld calls adj.set_value with the 578.7142160000001
> argument it synchronously receives value-changed event with same
> 578.7142160000001 argument (which is filtered out by meld's recursive call
> protection).
> 
> And then another asynchronous value-changed event with 578.0 value is
> received. Such asynchronous events always has zero decimal part, number is
> an integer. And those events are problem: unlike animation they are not
> reaction to user input, but are raised after meld's set_value.

Yep, you're totally right. However, the bug here *is* caused by animation (which you can easily check by toggling animation off with Tweak Tool or similar) because the coercion (which, by the way, I can't find either) happens on every step of the animation. Without animation we get the coercion once at the end, which doesn't actually cause a change in the scroll position of the other pane. Because it doesn't cause a change, we don't have to cycle back through the _sync_vscroll() logic.

> I've not found the gtk's code that performs coercion yet.
> But here is dirty workaround that resolves issue in my environment:
> just coerce value to integer inside meld before passing it to gtk.
> Attaching diff.

This isn't even a dirty workaround; it's pretty much the only actual fix I can find, short of convincing GTK+ to change their scroll behaviour. I've cleaned up the commit message (I hope you don't mind) and pushed this to master and the 3.16 branch.

If anyone still sees this bug (or something like it) with either current master or the current meld-3-16 branch, please add a comment with details (or open another bug).

Thanks very much for the patch.
Comment 16 Vasily Galkin 2016-09-17 10:04:26 UTC
The risk is that it is nowhere documented (and by now even is not extracted from current gtk sources) that the coercion is to integers. I can imagine that it is for example "to even integers on hidpi displays".

By the way I found the coercing code with gdb: it is gtk_text_view_set_vadjustment_values in gtktextview.c

It has a comment about the role coercion, which is not as simple that rounding to integer:
  /* Now adjust the value of the adjustment to keep the cursor at the
   * same place in the buffer */
Comment 17 Kai Willadsen 2016-09-17 21:54:59 UTC
Fair enough. I've just filed bug 771602 with a simple test case.
Comment 18 Kai Willadsen 2016-09-23 23:50:24 UTC
*** Bug 771878 has been marked as a duplicate of this bug. ***
Comment 19 Ed Adasiewicz 2016-10-02 14:37:27 UTC
I just upgraded one of my Arch systems to use the testing repo.  Part of this was to install meld 3.16.3-1 and gtk3 3.22.1-1.  The problem I originally reported no longer occurs.  Thanks for the fix.