Opened 9 years ago

Closed 7 years ago

#2050 closed Bug (fixed)

Properties dialog changes seem to undo themselves

Reported by: simonbcn Owned by: Longinus00
Priority: Normal Milestone: 2.00
Component: Qt Client Version: 1.60
Severity: Normal Keywords:
Cc:

Description

Example:
I open the "torrent properties" window of one torrent.
I limit upload speed:
Options > "Limit upload speed (KB/s)" = 50
I press "Close"
I wait a few seconds, I open again its "torrent properties" and "Limit upload speed" is disabled. ¿¿??
In addition, the upload speed is still well above 50.

By other hand, sometimes, when I try to change something in "torrent properties" window, my changes are deleted.
I suppose that's because it refreshes the window again.

Attachments (4)

qt_refresh.patch (5.8 KB) - added by Longinus00 8 years ago.
qt_refresh2.patch (2.8 KB) - added by Longinus00 8 years ago.
qt_refresh3.patch (1.6 KB) - added by Longinus00 8 years ago.
qt_refresh3.2.patch (1.6 KB) - added by Longinus00 8 years ago.
fix spaces

Download all attachments as: .zip

Change History (30)

comment:1 in reply to: ↑ description Changed 9 years ago by charles

  • Status changed from new to assigned
  • Summary changed from Changes in "Torrent Properties" window are not facts to Properties dialog changes seem to instantly reset themselves

Replying to simonbcn:

Example:
I open the "torrent properties" window of one torrent.
I limit upload speed:
Options > "Limit upload speed (KB/s)" = 50
I press "Close"
I wait a few seconds, I open again its "torrent properties" and "Limit upload speed" is disabled. ¿¿??
In addition, the upload speed is still well above 50.

When you set it to 50, the Qt client sends that request upstream to the transmission server. The dialog is greyed out because the Qt client is waiting for a refresh of information from the server so that it'll be showing up-to-date information.

By other hand, sometimes, when I try to change something in "torrent properties" window, my changes are deleted.
I suppose that's because it refreshes the window again.

Yeah, this is definitely a problem. Here's what's happening: when the properties dialog is open, the Qt client makes a request to the server every few seconds for all the information it needs to refresh the properties dialog. If you make a change while one of those refreshes is in transit, then the response was generated by the server *before* you made the change. The next refresh will correct the problem -- say, if you change the speed from 50 to 51, it will seem to jump back to 50, and then a few seconds later jump back to 51 again.

But this is definitely confusing behavior.

Probably what we should do is disregard the pending properties refresh messages from the server when a change is made in the gui.

Possible implementation:

  • add a timestamp to each of the properties dialog refresh requests sent to the server.
  • add an "ignore-before" timestamp to server.cc.
  • when a properties change is sent, set ignore-before to the current time.
  • when a properties refresh response comes back from the server, only use it if its timestamp is >= ignore-before.

comment:2 follow-up: Changed 9 years ago by simonbcn

Ok, you're right.
I change one property. I wait, with the properties window open, it updates the window with the old value but then it shows the value changed.
Yes, it's very confusing.

Thanks for the fast reply.

comment:3 in reply to: ↑ 2 Changed 9 years ago by charles

Replying to simonbcn:

Yes, it's very confusing.

I agree... it needs to be fixed before the Qt client goes from beta to stable

comment:4 Changed 9 years ago by simonbcn

If you need a beta-tester, I am ready.
My english is not very good, but it serves to understand.
In my case, the test environment is Linux Ubuntu 8.04 64-bit in server and Linux Ubuntu 9.04 32-bit in PC.

comment:5 Changed 8 years ago by charles

  • Summary changed from Properties dialog changes seem to instantly reset themselves to Properties dialog changes appear to reset themselves

comment:6 Changed 8 years ago by charles

  • Summary changed from Properties dialog changes appear to reset themselves to Properties dialog changes seem to undo themselves

comment:7 Changed 8 years ago by skotopes

I think it`s good idea to move some part of stuff from refresh() (including all stuff for speed tab redraw) to overloaded show() and probably it has sense to make propertys window application modal. I`ll try do it at sunday

comment:8 Changed 8 years ago by skotopes

Small hack for a first time. Should isolate Options tab against redraw.

http://pastebin.org/60779

comment:9 Changed 8 years ago by charles

skotopes: don't forget the Preferences dialog too ;)

comment:10 Changed 8 years ago by skotopes

http://pastebin.org/60963 cleanuped version of previous patch.

I spent some time on prefs code and think that it has sense to change windows behavior: for example we do not need to redraw each setting on single update, instead we can load them all on show. But its a lot of code changes, and im afraid to brake application codding and behavior style. So if you will accept this changes, i can make patch.

comment:11 Changed 8 years ago by charles

Hmmm...

The rationale for redrawing the settings on an update was that if some other source had made the change, then the change would be reflected in the GUI. Such as, if you had two copies of the Qt client connected to the same session and were tweaking its settings.

However, that kind of scenario is probably the kind of thing only programmers could dream up, that doesn't really happen in the real world. If by loading the settings only on show() we're trading this real bug #2050 for that hypothetical trick, IMO that's a good tradeoff.

Go for it. :)

comment:12 Changed 8 years ago by charles

skotopes: did you ever make that patch? I'd still be happy to use it.

comment:13 Changed 8 years ago by skotopes

charles: first part was published here http://trac.transmissionbt.com/ticket/2050#comment:10, but it looks like paste is already dead, second i posted in irc, but i do not remember where them.

i`ll try to find patches on my pc if they still exists(i am not sure that i kept them for 5 months)

Last edited 8 years ago by skotopes (previous) (diff)

comment:14 Changed 8 years ago by skotopes

patch for this bug: http://myau.su/transmission-qt-details-refresh.patch

patch for prefs is lost. fixing for prefs and other dialog windows (which uses asynchronous work with widgets) requires code refactoring and time. And of cause a wish to do this job, which i don`t have after complete migration on mac.

comment:15 Changed 8 years ago by Longinus00

skotopes patch has the side effect of having to close the details window each time you want to see the details of a different torrent.

My solution to the preferences dialog updating to disable it completely. If there is a key that actually should be updated that can be easily added in but I'm unsure if any need it.

Last edited 8 years ago by Longinus00 (previous) (diff)

Changed 8 years ago by Longinus00

comment:16 Changed 8 years ago by Longinus00

Actually, modifying my patch to allow file tree updating wasn't very had at all.

comment:17 Changed 8 years ago by charles

  • Milestone changed from None Set to 2.00
  • Owner changed from charles to Longinus00
  • Priority changed from High to Normal
  • Status changed from assigned to new

comment:18 Changed 8 years ago by charles

  • Resolution set to fixed
  • Status changed from new to closed

11:41:16 < CIA-40> charles * r10627 qt/ (5 files): (trunk Qt) #2050 "Properties dialog changes seem to undo themselves" -- use patch from Longinus00

Changed 8 years ago by Longinus00

comment:19 Changed 8 years ago by Longinus00

  • Resolution fixed deleted
  • Status changed from closed to reopened

The preferences dialog was never getting updated because it only gets updated at creation and it only got created once.

comment:20 Changed 8 years ago by charles

2nd patch applied to trunk in r10630.

Longinus00, if this issue is resolved now feel free to close this ticket. Thanks!

Changed 8 years ago by Longinus00

comment:21 Changed 8 years ago by Longinus00

The default behavior in qt is to hide a dialog when pressing the close button in the titlebar which can lead to values not getting updated.

Changed 8 years ago by Longinus00

fix spaces

comment:22 Changed 8 years ago by charles

qt_refresh3.2.patch committed to trunk for 2.0 by r10632

comment:23 Changed 8 years ago by charles

Longinus00, can this ticket be closed now?

comment:24 Changed 8 years ago by Longinus00

  • Resolution set to fixed
  • Status changed from reopened to closed

Yes, sorry about that.

comment:25 Changed 7 years ago by charles

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopened as per description of rb07 at https://trac.transmissionbt.com/ticket/3772#comment:6

comment:26 Changed 7 years ago by charles

  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed in r11492

Note: See TracTickets for help on using tickets.