Created attachment 118488 [details] [review]
patch to drop page_size from the GtkAdjustments

Hi,

Attached is the patch that fixes this for me.

From the changelog entry I wrote to accompany it for the package:

  * GtkAdjustment behaviour has changed to restrict the range of the values.
    From the GTK README:
      GtkAdjustment now enforces that values are restricted to the
      range [lower, upper - page_size]. This has always been the
      documented behaviour, and the recommended practice is to set
      page_size to 0 when using adjustments for simple scalar values,
      like in a slider or spin button.
    This change meant that the time couldn't be set to greater than 13:49:49,
    as the upper was 23:59:59 and the page_sizes 10:10:10.
    debian/patches/91_time_admin_adjustment.patch corrects this by reducing
    the page_sizes to 0, as recommended in the above quote.
    - http://bugzilla.gnome.org/show_bug.cgi?id=551740
    Thanks to Sebastien Bacher for finding the above information.

Thanks,

James

This is a rather severe regression.

I've recently updated to Intrepid from Hardy, which introduced Gtk+ 2.14.1, and many, many apps using GtkSpinbuttons (which are constructed using a GtkAdjustment) exhibit broken behaviour, among these is glade-2, where e.g. the X-Scale and Y-Scale widget properties can be only adjusted up to 0.9 instead of 1.0, and in Inkscape, where colors component values can not be set within in the full [0,255] interval but are limited to [0,245].


Explanation from Owen on IRC was: "GtkAdjustment was made to automatically clamp set values to max - page_size... [...]", which goes along with e.g. the respective widgets in glade-2 which have a "max" value of 1.0 and a "page size" value of 0.1.

However, i hope that it is not expected that all applications be fixed in this way as this is a rather collosal task to coordinate and huge work for the application authors.

In fact I'd consider this an API break, and a real regression and i vote for reverting the change for 2.14.2.

Thoughts?

This triggered a small collection of critical bugs in Ubuntu's installer: the manual partitioner could never have its partition size set to the maximum available, which also caused it to be confused when editing partitions, resize partitions without the user realising they'd asked it to, etc.

Of course, the fix was simple, and I uploaded it as soon as I realised the problem. However, like the previous commenter, I'm worried that similar bugs might be lurking elsewhere. Since the Ubuntu installer uses PyGTK, I generally only ever used the excellent PyGTK documentation and didn't look at the GTK+ documentation (for the most part, admonishments in the GTK+ documentation are reproduced for PyGTK developers and I've normally found the PyGTK documentation alone to be quite sufficient). However, the description of gtk.Adjustment (http://www.pygtk.org/docs/pygtk/class-gtkadjustment.html) contains no indication that values will be restricted to upper - page_size, and simply documents upper as "the maximum value" without qualification. Thus, I think we can expect misuse of page_size to be widespread in PyGTK code.

Even the GTK+ documentation itself is either incorrect or at best unhelpful. Here's the only relevant bit I can find about page_size:

  Sets the GtkAdjustment value. The value is clamped to lie between adjustment->lower and adjustment->upper.

  Note that for adjustments which are used in a GtkScrollbar, the effective range of allowed values goes from adjustment->lower to adjustment->upper - adjustment->page_size.

That doesn't make it at all clear that adjustments work the way the release notes say - and this is what the documentation says in 2.14 as well as in 2.12! In fact, the first paragraph seems to explicitly contradict the release notes, and the second paragraph only repeals that for adjustments used in scrollbars.

The only place I can find where this is actually described correctly is in the documentation comments in gtkadjustment.c:

  /**
   * GtkAdjustment:upper:
   *
   * The maximum value of the adjustment.
   * Note that values will be restricted by
   * <literal>upper - page-size</literal> if the page-size
   * property is nonzero.
   *
   * Since: 2.4
   */

  /**
   * GtkAdjustment:page-size:
   *
   * The page size of the adjustment.
   * Note that the page-size is irrelevant and should be set to zero
   * if the adjustment is used for a simple scalar value, e.g. in a
   * #GtkSpinButton.
   *
   * Since: 2.4
   */

These do indeed match what the release notes say. However, I think we could forgive application developers for missing this, since none of this is reflected in the HTML documentation which most of them will be reading (assuming they're reading any documentation at all, but I don't care about the ones who don't).

Could we please revert this change and get the documentation updated, then restore the clamping in a few releases' time? That would be better than just breaking the unwitting offenders.

Sorry, I should point out that by "this" I meant the GTK+ change. And I've just realised that this bug is on gnome-system-tools, not GTK+ ...

Pushed as 1a96d1c8a43efe255740341c6819c398c95b2d4f.
Thanks for the patch, James! It should be included in g-s-t 2.27.2.

*** Bug 584207 has been marked as a duplicate of this bug. ***