<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/110914/">https://git.reviewboard.kde.org/r/110914/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 15th, 2014, 11:33 p.m. UTC, <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;">Can you investigate this?
Open two files in two tabs:
* Go to tab 2
* Press Ctrl+F
* Press Esc
* Search bar closes
* Press Ctrl+F
* Go to tab 1
* Go to tab 2
* Press Esc
* Search bar does not close
</pre>
</blockquote>
<p>On January 16th, 2014, 6:03 a.m. UTC, <b>Jonathan Doman</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;">As you may have noticed, the problem is that the Part::m_closeFindBar action loses its shortcut (Key_Escape) after switching tabs. However, I can't figure out why this happens. The shortcut seems to be lost/reset after calling createGUI(). Commit f81a49fa (see https://git.reviewboard.kde.org/r/102358/) introduced this behavior, because it moved the shortcut setting from a static initialization in Part::setupViewerAction() to an on/off toggle that happens when the findBar is shown and hidden. For some reason, the static initialization is necessary. If I can figure out how to build kdelibs, I can dig deeper, but this isn't directly related to my tabs code.</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;">What do you mean it's not related to your tabs code? It is something that works without your patch and doesn't work with your patch. So yes it is related to your code. Maybe you need to add more, like tabswitching meaning re-creating the shortcut or something, but it is something that needs to be fixed before we merge this feature in.</pre>
<br />
<p>- Albert</p>
<br />
<p>On January 13th, 2014, 12:24 a.m. UTC, Jonathan Doman wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Jonathan Doman.</div>
<p style="color: grey;"><i>Updated Jan. 13, 2014, 12:24 a.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=155515">155515</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;">This patch adds support for a tabbed interface (multiple documents in one window). The core work just adds a tab bar that switches between multiple embedded okularparts, but there are many other considerations:
- Tab context menu allows for duplicating or detaching (detached tabs start in new okular process)
- `okular file.pdf` will open file in existing window if possible, unless --new flag is used. It also selects the most recently raised/activated window to use. This mirrors behavior I expect from browsers and other tabbed interfaces.
- Warns when closing window with multiple tabs
- No warning is given when opening an already open file. This is the behavior I strongly prefer (and observe in other programs), but will change if there is consensus otherwise.
When selecting different tools in one part, the tool selection propagates to all parts, but the GUI does not reflect that. This bug is present in other programs (e.g. multiple okularparts in Konqueror), so I made no attempt to diagnose or fix.
One menu item was added for the multiple tab warning option. When testing this, I noticed that items in the Settings menu seem to move around when switching tabs, and I cannot diagnose or fix this. It seems to be related to XMLGUI bug #64754.
My development branch is also hosted at https://github.com/jrmrjnck/okular-tabbed</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>part.h <span style="color: grey">(4b3aafdb637080ae81eb0e45742f53a34738984d)</span></li>
<li>part.cpp <span style="color: grey">(88e2c41059344dbd11d4c94f99c63a3f5bc8c99b)</span></li>
<li>shell/shell.h <span style="color: grey">(c065c560fb4ddfcf181601cf35e9ca14581731f6)</span></li>
<li>shell/shell.cpp <span style="color: grey">(1708501daaef817a1ce35fa5d96701a66ab66983)</span></li>
<li>shell/shell.rc <span style="color: grey">(93fbc417588312792bab39b693c65e5d414c87c6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/110914/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>