Gwibber

Merge lp:~inash/gwibber/gwibber-425929 into lp:gwibber

  1. gwibber-425929
  2. Merge into trunk
Proposed by Inash Zubair
Status: Rejected
Rejected by: Robert Bruce Park
Proposed branch: lp:~inash/gwibber/gwibber-425929
Merge into: lp:gwibber
Diff against target: 12 lines (+1/-1)
1 file modified
gwibber/gwui.py (+1/-1)
To merge this branch: bzr merge lp:~inash/gwibber/gwibber-425929
Reviewer Review Type Date Requested Status
Robert Bruce Park Disapprove
Bilal Akhtar (community) Approve
gwibber-committers Pending
Review via email: mp+22772@code.launchpad.net

Description of the change

fix for bug #425929: if application font size is a float (eg: 8.5999), gwibber crashes with exception.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

Does it really need to eventually be cast to an int? I removed that and let it just use a float and that worked for me.

Otherwise it looks correct to me -- please merge it.

=== modified file 'gwibber/gwui.py'
--- gwibber/gwui.py 2010-03-31 04:22:12 +0000
+++ gwibber/gwui.py 2010-05-10 14:34:08 +0000
@@ -237,7 +237,7 @@
     default_font = self.gc.get_string("/desktop/gnome/interface/font_name")
     font_name, font_size = default_font.rsplit(None, 1)
     self.web_settings.set_property("sans-serif-font-family", font_name)
- self.web_settings.set_property("default-font-size", int(font_size))
+ self.web_settings.set_property("default-font-size", float(font_size))

     if not resources.theme_exists(theme):
       theme = "default"

Revision history for this message
Inash Zubair (inash) wrote :

Hi Martin,

I guess both are correct. It boils down to using just float because that would eventually have a more correct and accurate value from the global application font size. Shall I patch it up as you've stated and re-propose for merging?

Revision history for this message
Omer Akram (om26er) wrote :

This also need to be merged to gwibber 2.30 branch.

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Looks good.

Some comitter, please merge.

review: Approve
Revision history for this message
Robert Bruce Park (robru) wrote :

Thanks for taking the time to submit this patch, unfortunately Gwibber has gone through extensive changes recently and your patch no longer applies to the latest codebase.

Fortunately, the bug you were attempting to fix is no longer present in the current codebase. As we have switched to using Qml, the UI code is all radically different and font sizes are handled for us.

Please try out the latest Gwibber, I think you'll be quite pleased with what we've accomplished.

review: Disapprove

Unmerged revisions

707. By Inash Zubair

fixed application font size setting exception.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'gwibber/gwui.py'
2--- gwibber/gwui.py 2010-03-31 04:22:12 +0000
3+++ gwibber/gwui.py 2010-04-04 00:22:14 +0000
4@@ -237,7 +237,7 @@
5 default_font = self.gc.get_string("/desktop/gnome/interface/font_name")
6 font_name, font_size = default_font.rsplit(None, 1)
7 self.web_settings.set_property("sans-serif-font-family", font_name)
8- self.web_settings.set_property("default-font-size", int(font_size))
9+ self.web_settings.set_property("default-font-size", int(float(font_size)))
10
11 if not resources.theme_exists(theme):
12 theme = "default"