<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/115283/">https://git.reviewboard.kde.org/r/115283/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On December 15th, 2014, 2:28 a.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This looks pretty solid now :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Two minor things:
 * You should add it to the manual, i.e. add something to doc/index.docbook
 * I'm not convinced of it's position on the menus. At the moment it's close to other "fit" things, but those are permanent, while this is an instant action and it works over the window size and not over the zoom value like the others. I'm undecided about two options: A) Don't show it on the menus at all and give it a shortcut so people can use it (Since i think this is a pretty "advanced/notcommon" option) B) Put it at the end of the View menu in it's own "group" (i.e. with a separator). What do you think?</p></pre>
 </blockquote>




 <p>On January 1st, 2015, 3:19 p.m. IST, <b>Saheb Preet Singh</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Before deciding its position on menus I have a doubt. I reviewed the bug description and found this</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">'Zoom in "fit window to page" mode causes the whole window to magnify or shrink accordingly.'</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Should I work on implementing the movement of window on zoom in and zoom out ?</p></pre>
 </blockquote>





 <p>On January 3rd, 2015, 12:21 a.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I honestly think that it's not needed.</p></pre>
 </blockquote>





 <p>On January 8th, 2015, 2:43 p.m. IST, <b>Saheb Preet Singh</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">i think keeping it as a shortcut would be a good idea</p></pre>
 </blockquote>





 <p>On January 9th, 2015, 3:26 a.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now we need to think of a default shortcut. Any suggestion?</p></pre>
 </blockquote>





 <p>On January 26th, 2015, 10:13 a.m. IST, <b>Saheb Preet Singh</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ctrl + E or ctrl + shift + F</p></pre>
 </blockquote>





 <p>On January 27th, 2015, 12:38 a.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ctrl + shift + F is obviously out of question since it's already used</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's the rationale behind Ctrl+E?</p></pre>
 </blockquote>





 <p>On January 27th, 2015, 1:14 p.m. IST, <b>Saheb Preet Singh</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">oh I am sorry for suggesting ctrl + shift + F. We can use something like ctrl + alt + F.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I suggested ctrl + E as it is already being used in gimp for shrink wrap.</p></pre>
 </blockquote>





 <p>On January 28th, 2015, 5:26 a.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hmmm, my gimp uses Ctrl+J ? Which gimp version do you use?</p></pre>
 </blockquote>





 <p>On February 21st, 2015, 2:53 p.m. IST, <b>Saheb Preet Singh</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">oops sorry, it works with ctrl + j only. So can we use ctrl + j ?</p></pre>
 </blockquote>





 <p>On February 22nd, 2015, 10 p.m. IST, <b>Albert Astals Cid</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ctrl+J seems good, can you make it the patch before this wednesday that is the Feature Freeze for the next release? If not please tell and i'll try to find time to do it myself.</p></pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">i am really very sorry for the late reply. I tried my best to do it till wednesday but couldn't do it. It will take me two to three days more to create the final patch.</p></pre>
<br />










<p>- Saheb</p>


<br />
<p>On November 25th, 2014, 12:47 p.m. IST, Saheb Preet Singh wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
 <tr>
  <td>

<div>Review request for Okular and Albert Astals Cid.</div>
<div>By Saheb Preet Singh.</div>


<p style="color: grey;"><i>Updated Nov. 25, 2014, 12:47 p.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=326844">326844</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">okular do not have a feature to resize the current window to the current page size.

The bug is solved upto some extent, the window is resizing as desired.

resize method of qwidget class is used to resize the window.

The window is resizing  as required but sometimes it doesn't resizes . It has to be moved by mouse and then again click on the button in the menu to resize the window again.  Once it is moved by mouse, it works fine. Now, when the zoom level is changed, the problem starts again.

I think the problem occurs due to something happening in the other portion of the code.

I searched about it on the internet and nobody was able to answer the solution to people with same problem. There are some alternative solutions to the problem:-

==> Use set geometry instead of resize which works fine. but would have to be taken care of the size of the screen and the original position of the window. Also, repainting the portion outside the window when the window is resized to a smaller window.

==> Use a timer which will signal a slot which in turn will resize the window, but that might require qsignalmapper to map the width and height to the slot. ( I didn't tried this solution ).

The best solution i could find on the internet is this http://qt-project.org/forums/viewthread/25648

Also, there is one more issue, the fit window to page button in the view menu is enabled by default. It should not be enabled when no document is open. Where should the code for enabling this button should be implemented.
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">
tested when
==> window is maximized and restored.
==> thumbnails, table of content are opened and closed.
==> various toolbars are shown and hidden.
==> for all the zoom levels shown in the toolbar and custom zoom level ( example 123.23% ).</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>shell/shell.cpp <span style="color: grey">(f7675fd)</span></li>

 <li>ui/pageview.h <span style="color: grey">(a8adb2c)</span></li>

 <li>ui/pageview.cpp <span style="color: grey">(17e66f4)</span></li>

 <li>ui/sidebar.h <span style="color: grey">(cf5dfc6)</span></li>

 <li>ui/sidebar.cpp <span style="color: grey">(0c722a9)</span></li>

 <li>part.h <span style="color: grey">(594eb44)</span></li>

 <li>part.cpp <span style="color: grey">(50a1b20)</span></li>

 <li>part.rc <span style="color: grey">(60f86e5)</span></li>

 <li>shell/shell.h <span style="color: grey">(224acfe)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/115283/diff/" style="margin-left: 3em;">View Diff</a></p>






  </td>
 </tr>
</table>







  </div>
 </body>
</html>