Re: [Okular-devel] Review Request 123680: Fix embed mode detection
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123680/ --- (Updated May 13, 2015, 10:59 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Changes --- Submitted with commit 223092aa0e1fd5fd1b48a34702d2102fdb1acccf by Albert Astals Cid on behalf of Jonathan Doman to branch Applications/15.04. Repository: okular Description --- Wow, I have no idea how this happened. I wrote an big test case and spent a lot of time playing with tabs. Stephan Binner even told me about this a month ago, but I didn't quite understand the problem... Broken embed mode detection means that the tabbed interface is disabled. It actually made it into release... ugh Diffs - core/script/kjs_document.cpp 4422953b0e157a9b0554a949969aaf9b13f44974 part.cpp 5b03e56e194bf84a9f02ceaf94cad13526715157 Diff: https://git.reviewboard.kde.org/r/123680/diff/ Testing --- Tabs work again. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 123680: Fix embed mode detection
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123680/ --- Review request for Okular. Repository: okular Description --- Wow, I have no idea how this happened. I wrote an big test case and spent a lot of time playing with tabs. Stephan Binner even told me about this a month ago, but I didn't quite understand the problem... Broken embed mode detection means that the tabbed interface is disabled. It actually made it into release... ugh Diffs - core/script/kjs_document.cpp 4422953b0e157a9b0554a949969aaf9b13f44974 part.cpp 5b03e56e194bf84a9f02ceaf94cad13526715157 Diff: https://git.reviewboard.kde.org/r/123680/diff/ Testing --- Tabs work again. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated March 16, 2015, 10:56 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Changes --- Submitted with commit 1cfb007b6339fa51c010fdd27e8de4838dadb5a5 by Albert Astals Cid on behalf of Jonathan Doman to branch Applications/15.04. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 tests/mainshelltest.cpp c5d7289d668f8a1ea0250deb068a43c19490edff Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 28, 2015, 4:41 p.m.) Review request for Okular. Changes --- Add unit test for session restoring. I couldn't figure a way to test the realistic full code paths of session restoring, but this should test the important parts inside okular. Currently the test just checks that the correct number of windows and tabs are restored. Overall I'm pretty happy with this now. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs (updated) - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 tests/mainshelltest.cpp c5d7289d668f8a1ea0250deb068a43c19490edff Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 28, 2015, 4:45 p.m.) Review request for Okular. Changes --- Remove some dev logic Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs (updated) - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 tests/mainshelltest.cpp c5d7289d668f8a1ea0250deb068a43c19490edff Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 22, 2015, 2:54 p.m.) Review request for Okular. Changes --- Clear tab list in slotQuit just in case it gets called twice. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs (updated) - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
On Feb. 22, 2015, 10:28 a.m., Albert Astals Cid wrote: Was thinking about the saving the viewport, are we closing the documents properly? Because if we are, on opening they should just be resotred to their proper viewport as when just opening a document from scratch, no? Yes, that's what I've been saying. This change properly closes the documents so that it's not necessary to save viewports in the session config. On Feb. 22, 2015, 10:28 a.m., Albert Astals Cid wrote: shell/shell.cpp, line 680 https://git.reviewboard.kde.org/r/122570/diff/2/?file=350709#file350709line680 Do we need to call slotQuit both from here and from the signal connection? I understand you mean session only calls aboutToQuit and not the closeEvent (which is weird), but does normal close only call closeEvent and not aboutToQuit? Yes, there are two different paths: 1. Window is closed by user. This triggers closeEvent (and calls destructor). Once windows are gone, app quits and then aboutToQuit is triggered. 2. Session manager closes application (through QCoreApplication::quit() I think). This just causes app.exec() to return - windows are not closed nor are destructors called. So the only way to respond is to connect to aboutToQuit. Just in case this changes in the future, we can clear the tab list in slotQuit so that it's okay to call twice. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/#review76430 --- On Feb. 22, 2015, 2:54 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 22, 2015, 2:54 p.m.) Review request for Okular. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 21, 2015, 11:10 p.m.) Review request for Okular. Changes --- Ensure that viewport is saved by properly closing each Part (`part-closeUrl()`). When the application is quit during session tear down, the Shell destructor is not called, so the same logic is now triggered by `QApplication::aboutToQuit` signal. Also a few cleanup changes: 1. RESTORE macro is replaced with preferred kRestoreMainWindows function. [ref](http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/kmainwindow_8h.html#a59a5929cb898cca8ac26a3d55397aae3) 2. Shell object name is made unique as recommended by docs [ref](http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKMainWindow.html#ab2f7f5f30e920d8490f3f83fe46149d0) 3. Remove .moc include since it isn't necessary and breaks my editing workflow (compiler-enabled code completion doesn't work when moc file is outside source tree). Still have yet to as tests. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs (updated) - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/main.cpp 16289608f0acf299db04258d842bbb87add62c0b shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
On Feb. 16, 2015, 4:43 p.m., Albert Astals Cid wrote: How much work would be to add to autotests the manual tests you mention? No idea, never written qt tests before. I'll take a look. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/#review76161 --- On Feb. 14, 2015, 8:31 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 14, 2015, 8:31 p.m.) Review request for Okular. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
On Feb. 16, 2015, 4:42 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 332 https://git.reviewboard.kde.org/r/122570/diff/1/?file=349746#file349746line332 We're not saving the viewport anymore? See the description: I tried to save the viewport, but the code became very complicated, and I couldn't get it working anyway. The viewport is already saved during a normal close, so I will try to figure out how to invoke the same logic during a session shut down. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/#review76160 --- On Feb. 14, 2015, 8:31 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- (Updated Feb. 14, 2015, 8:31 p.m.) Review request for Okular. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 122570: Update session restore/save to account for multiple tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122570/ --- Review request for Okular. Bugs: 335852 http://bugs.kde.org/show_bug.cgi?id=335852 Repository: okular Description --- New Shell logic loops through each tab and saves URLs and active tab index in session config. Viewport was previously saved in session config, but I opted to remove it because: 1. It complicates the restore logic. It would require either using QMetaObject::invoke or adding functions to ViewerInterface. Also hard to get right since opening a document isn't exactly synchronous. 2. Viewport info is already saved during a graceful shutdown. Diffs - part.h 594eb44113ae130a6fefbf2800af32886aa3cbef part.cpp 36438af1cd1036ee954f80b5359a0cab2c019036 shell/shell.h 224acfe023ef8e9cc58b52ddf32068af8937896a shell/shell.cpp f7675fdc8203e90210b8ba82620b19ae69ee43e1 Diff: https://git.reviewboard.kde.org/r/122570/diff/ Testing --- I was not familiar with session functionality in KDE before working on this bug, so my tests may not represent reality. I used the dbus interface to trigger a session save (org.kde.KSMServerInterface.saveCurrentSession), and then reloaded a session by running `okular --session xyz`, which I think is how KDE does it behind the scenes. - Restore one or more documents in single window with tabs enabled. - Restore multiple windows, tabs enabled or disabled. - Restore session config describing multible tabs, even though tabs are disabled Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 335852] Multiple tabs session restore fails
https://bugs.kde.org/show_bug.cgi?id=335852 --- Comment #5 from Jonathan Doman jonathan.do...@gmail.com --- I'm trying to work on this bug but hitting some roadblocks due to the Part plugin design. The Shell needs to get and set some information on each Part (the viewport), and this can't really be done using signals/slots. Can I add appropriate functions to the ViewerInterface? -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 120660: Allow each PageView to use a different tool
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120660/ --- (Updated Dec. 10, 2014, 2:34 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Bugs: 334251 http://bugs.kde.org/show_bug.cgi?id=334251 Repository: okular Description --- Keep a local MouseMode setting, and don't rely on the value returned by Settings::mouseMode(). In the bug report, I stated that each tab should be allowed to use different tools. No one objected, but after testing it's not clear this is the right answer. The existing behavior was to force all tabs/windows to use the same tool, but then it's not clear what to do with review mode. Diffs - ui/pageview.cpp 17e66f4a3b420bbcaf281532fa9d84379c74d48c Diff: https://git.reviewboard.kde.org/r/120660/diff/ Testing --- Followed steps in bug report for both tabs and windows to make sure different tools are allowed. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 120660: Allow each PageView to use a different tool
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120660/ --- Review request for Okular. Bugs: 334251 http://bugs.kde.org/show_bug.cgi?id=334251 Repository: okular Description --- Keep a local MouseMode setting, and don't rely on the value returned by Settings::mouseMode(). In the bug report, I stated that each tab should be allowed to use different tools. No one objected, but after testing it's not clear this is the right answer. The existing behavior was to force all tabs/windows to use the same tool, but then it's not clear what to do with review mode. Diffs - ui/pageview.cpp 17e66f4a3b420bbcaf281532fa9d84379c74d48c Diff: https://git.reviewboard.kde.org/r/120660/diff/ Testing --- Followed steps in bug report for both tabs and windows to make sure different tools are allowed. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334251] Tools don't work if any tab is in review mode
https://bugs.kde.org/show_bug.cgi?id=334251 --- Comment #6 from Jonathan Doman jonathan.do...@gmail.com --- So I tested an older version (tag v4.10.97) and this bug exists for multiple windows as well. It's probably been around forever. And there are similar bugs for other settings like continuous view mode. Basically I think the global Settings object is very inadequate for how it's used, and should be re-architected. I guess I'll keep trying to find a short term solution, though. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334251] Tools don't work if any tab is in review mode
https://bugs.kde.org/show_bug.cgi?id=334251 --- Comment #3 from Jonathan Doman jonathan.do...@gmail.com --- I took some time to look at this and I'm not sure how to fix it. The problem is that changing the mouse mode writes the global Settings, which triggers all the Parts to reload (and potentially overwrite) the Settings. We could: 1. Don't watch for the Settings::configChanged signal. This is my preference, as I wouldn't expect the program to reload settings whenever the config file is written to. 2. Don't constantly write settings to disk. Most Settings mutations are followed by a writeConfig(), which seems unnecessary to me. Why not just writeConfig() once when the program is exiting? 3. Remove MouseMode as a global setting. Why not just always start the program in Browse mode? I would ultimately prefer to get rid of all persistent, implicit settings and make everything non-configurable or explicitly configurable because I don't think there's an intuitive way to resolve settings when multiple program instances are simultaneously altering them. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 119595: Pass the command line options properly when using tabs or unique instances
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119595/#review63990 --- Ship it! Haven't had time to build and test, but the code looks good to me. - Jonathan Doman On Aug. 3, 2014, 3 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119595/ --- (Updated Aug. 3, 2014, 3 p.m.) Review request for Okular and Jonathan Doman. Bugs: 334100 http://bugs.kde.org/show_bug.cgi?id=334100 Repository: okular Description --- Use a QString to serialize the command line options other than urls to open so it can be easily passed around to every place that opens a new shell or tab or overrides the content in a unique instance. Maybe there's something the autotest does not test but it'd be a cornercase, I'd like to have this for 4.14 Diffs - shell/main.cpp 61e2113 shell/okular_main.h PRE-CREATION shell/okular_main.cpp PRE-CREATION shell/shell.h e540054 shell/shell.cpp 55f4f12 shell/shellutils.h 6c0c228 shell/shellutils.cpp 26c6825 tests/CMakeLists.txt 3b3fbdd tests/mainshelltest.cpp PRE-CREATION conf/settings.kcfgc 9064d15 core/document.cpp 921a7e8 core/document_p.h aabd192 shell/CMakeLists.txt 521a216 Diff: https://git.reviewboard.kde.org/r/119595/diff/ Testing --- I have an autotest! Thanks, Albert Astals Cid ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 119550: Use absolute filepath when attaching to existing windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119550/ --- (Updated Aug. 3, 2014, 12:06 a.m.) Status -- This change has been marked as submitted. Review request for Okular. Bugs: 334510 http://bugs.kde.org/show_bug.cgi?id=334510 Repository: okular Description --- When opening files through DBus, relative file names don't work if the processes are in different working directories. Ensure that all arguments are made absolute. Diffs - shell/main.cpp d2e0ec72fd43b560d25ac7f903462641cdbbff51 Diff: https://git.reviewboard.kde.org/r/119550/diff/ Testing --- Tested steps described in bug report. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 119550: Use absolute filepath when attaching to existing windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119550/ --- (Updated Aug. 1, 2014, 6:32 p.m.) Review request for Okular. Changes --- Account for remote URLs. This is somewhat heavy-handed and duplicates work, but I'm not sure how else to do it while preserving the existing DBus interface. Bugs: 334510 http://bugs.kde.org/show_bug.cgi?id=334510 Repository: okular Description --- When opening files through DBus, relative file names don't work if the processes are in different working directories. Ensure that all arguments are made absolute. Diffs (updated) - shell/main.cpp d2e0ec72fd43b560d25ac7f903462641cdbbff51 Diff: https://git.reviewboard.kde.org/r/119550/diff/ Testing --- Tested steps described in bug report. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 118685: Make tabs rearrangeable
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118685/ --- (Updated July 30, 2014, 10:15 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Bugs: 334018 http://bugs.kde.org/show_bug.cgi?id=334018 Repository: okular Description --- Catch the tabMoved signal emitted by the tab bar in order to rearrange the internal data. Slightly more than the single line I thought it would be. Diffs - shell/shell.h f25b3d8c01215534b9a7097d1e229607c8f98ef3 shell/shell.cpp 9ee422a60feb31286abc5ec178ce64835fd80781 Diff: https://git.reviewboard.kde.org/r/118685/diff/ Testing --- Tested that the GUI remains coherent when moving two tabs around many times. As before, the tool selections aren't quite coherent, but that's for another patch. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 119550: Use absolute filepath when attaching to existing windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119550/ --- Review request for Okular. Bugs: 334510 http://bugs.kde.org/show_bug.cgi?id=334510 Repository: okular Description --- When opening files through DBus, relative file names don't work if the processes are in different working directories. Ensure that all arguments are made absolute. Diffs - shell/main.cpp d2e0ec72fd43b560d25ac7f903462641cdbbff51 Diff: https://git.reviewboard.kde.org/r/119550/diff/ Testing --- Tested steps described in bug report. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 119550: Use absolute filepath when attaching to existing windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119550/ --- (Updated July 30, 2014, 11:50 p.m.) Review request for Okular. Bugs: 334510 http://bugs.kde.org/show_bug.cgi?id=334510 Repository: okular Description --- When opening files through DBus, relative file names don't work if the processes are in different working directories. Ensure that all arguments are made absolute. Diffs (updated) - shell/main.cpp d2e0ec72fd43b560d25ac7f903462641cdbbff51 Diff: https://git.reviewboard.kde.org/r/119550/diff/ Testing --- Tested steps described in bug report. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334510] Opening a second file from shell with a relative folder name when tabs are used fails
https://bugs.kde.org/show_bug.cgi?id=334510 --- Comment #3 from Jonathan Doman jonathan.do...@gmail.com --- Thanks for the tip, Willem. The patch is posted. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 118685: Make tabs rearrangeable
On July 15, 2014, 2:27 p.m., Albert Astals Cid wrote:
shell/shell.cpp, line 114
https://git.reviewboard.kde.org/r/118685/diff/1/?file=280425#file280425line114
What do you mean KTabWidget does not expose the underlying tabbar?
KTabWidget is a QTabWidget and is setting its tabbar with
setTabBar( new KTabBar( this ) );
so how would QTabWidget::tabBar not return something valid?
That function is protected. I would have to subclass {K,Q}TabWidget to gain
access to it.
- Jonathan
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118685/#review62426
---
On June 11, 2014, 11:52 p.m., Jonathan Doman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118685/
---
(Updated June 11, 2014, 11:52 p.m.)
Review request for Okular.
Bugs: 334018
http://bugs.kde.org/show_bug.cgi?id=334018
Repository: okular
Description
---
Catch the tabMoved signal emitted by the tab bar in order to rearrange the
internal data. Slightly more than the single line I thought it would be.
Diffs
-
shell/shell.h f25b3d8
shell/shell.cpp 9ee422a
Diff: https://git.reviewboard.kde.org/r/118685/diff/
Testing
---
Tested that the GUI remains coherent when moving two tabs around many times.
As before, the tool selections aren't quite coherent, but that's for another
patch.
Thanks,
Jonathan Doman
___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 118685: Make tabs rearrangeable
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118685/ --- (Updated July 15, 2014, 9:52 p.m.) Review request for Okular. Changes --- Whaddya know, there is a KTabWidget::tabBar! Bugs: 334018 http://bugs.kde.org/show_bug.cgi?id=334018 Repository: okular Description --- Catch the tabMoved signal emitted by the tab bar in order to rearrange the internal data. Slightly more than the single line I thought it would be. Diffs (updated) - shell/shell.h f25b3d8c01215534b9a7097d1e229607c8f98ef3 shell/shell.cpp 9ee422a60feb31286abc5ec178ce64835fd80781 Diff: https://git.reviewboard.kde.org/r/118685/diff/ Testing --- Tested that the GUI remains coherent when moving two tabs around many times. As before, the tool selections aren't quite coherent, but that's for another patch. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 118685: Make tabs rearrangeable
On July 14, 2014, 5:45 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 114 https://git.reviewboard.kde.org/r/118685/diff/1/?file=280425#file280425line114 Please just call tabBar() If only it were so easy - KTabWidget does not expose the underlying tab bar. Personally, I find this simpler than subclassing KTabWidget or using KTabBar+QStackedLayout. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118685/#review62354 --- On June 11, 2014, 11:52 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118685/ --- (Updated June 11, 2014, 11:52 p.m.) Review request for Okular. Bugs: 334018 http://bugs.kde.org/show_bug.cgi?id=334018 Repository: okular Description --- Catch the tabMoved signal emitted by the tab bar in order to rearrange the internal data. Slightly more than the single line I thought it would be. Diffs - shell/shell.h f25b3d8 shell/shell.cpp 9ee422a Diff: https://git.reviewboard.kde.org/r/118685/diff/ Testing --- Tested that the GUI remains coherent when moving two tabs around many times. As before, the tool selections aren't quite coherent, but that's for another patch. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 335852] Multiple tabs session restore fails
https://bugs.kde.org/show_bug.cgi?id=335852 --- Comment #2 from Jonathan Doman jonathan.do...@gmail.com --- Okay, got it. I'm settled in a new city now, so I'll put up patches as fast as you can review them. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 118685: Make tabs rearrangeable
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118685/ --- Review request for Okular. Bugs: 334018 http://bugs.kde.org/show_bug.cgi?id=334018 Repository: okular Description --- Catch the tabMoved signal emitted by the tab bar in order to rearrange the internal data. Slightly more than the single line I thought it would be. Diffs - shell/shell.h f25b3d8 shell/shell.cpp 9ee422a Diff: https://git.reviewboard.kde.org/r/118685/diff/ Testing --- Tested that the GUI remains coherent when moving two tabs around many times. As before, the tool selections aren't quite coherent, but that's for another patch. Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334251] New: Tools don't work if any tab is in review mode
https://bugs.kde.org/show_bug.cgi?id=334251 Bug ID: 334251 Summary: Tools don't work if any tab is in review mode Classification: Unclassified Product: okular Version: 0.19.0 Platform: Archlinux Packages OS: Linux Status: UNCONFIRMED Severity: normal Priority: NOR Component: general Assignee: okular-devel@kde.org Reporter: jonathan.do...@gmail.com The non-browse tools (zoom, select, etc.) don't work if any tab in the window is in review mode. Normally, a single tab will enforce the mutual exclusivity of review mode and non-browse tools. But there is some kind of interaction between tabs that causes all tools in all tabs to behave as a browse tool without properly reflecting this in the GUI. I'm happy to fix this but it may be over a month before I have time to do so. Reproducible: Always Steps to Reproduce: 1. Open two documents in tabs 2. Enter review mode in doc 1. (At this point, doc 1 will enforce the use of browse tool.) 3. Switch to doc 2 and try to use the zoom tool (or any other tool). Actual Results: The zoom tool will be selected in the GUI and the corresponding tooltip will show, but in fact the browse tool is still active. Expected Results: Each tab should be allowed to use different tools, regardless of whether other tabs are in review mode. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334100] --page number being ignored
https://bugs.kde.org/show_bug.cgi?id=334100 Jonathan Doman jonathan.do...@gmail.com changed: What|Removed |Added CC||jonathan.do...@gmail.com --- Comment #1 from Jonathan Doman jonathan.do...@gmail.com --- Thanks for reporting this - I can confirm that I broke this functionality. In fact, I think many of the command line options won't work properly. I'm happy to fix this, but it may be over a month until I have time to do so. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334019] Okular should warn if closing with multiple tabs opened
https://bugs.kde.org/show_bug.cgi?id=334019 --- Comment #2 from Jonathan Doman jonathan.do...@gmail.com --- I'm happy to implement this, but it may be a month before I have time to do it. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 334018] Tabs should be rearrangeable
https://bugs.kde.org/show_bug.cgi?id=334018 --- Comment #1 from Jonathan Doman jonathan.do...@gmail.com --- I'm happy to implement this, but it may be a month before I have time to do it. It's really just one line of code, but there are a few other tab improvements I'd like to do at the same time. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated April 9, 2014, 10:08 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Bugs: 331872 and 332238 http://bugs.kde.org/show_bug.cgi?id=331872 http://bugs.kde.org/show_bug.cgi?id=332238 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 part.h 010e9de1f2b5c27531a48b943d821ccc3f3f7205 part.cpp 4ce7e28e1071772921e6292e61a88c905a62d7f6 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 ui/thumbnaillist.h 61601c228512772fd46abc27468523aef562c3fa ui/sidebar.cpp 2474db8c357f6bfd1a9b6bd75091f2eb7b7b7693 ui/pageview.cpp dd4199450672c18ebfa146327d8e9b7eb034ddc8 ui/sidebar.h 036d7788096366d6bab7d7f2a41d55b7a31d303a ui/pageview.h 577b908633bd0778df33d0a15961ab16071a1500 ui/thumbnaillist.cpp 72b557e6624e8229cf1e5e4c5dc69ed77fec54cb Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 20, 2014, 2:33 p.m.) Review request for Okular. Changes --- Handle multiple command line arguments when there is no existing window. And accept DD on the tab bar. Bugs: 331872 and 332238 http://bugs.kde.org/show_bug.cgi?id=331872 http://bugs.kde.org/show_bug.cgi?id=332238 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs (updated) - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 part.h 010e9de1f2b5c27531a48b943d821ccc3f3f7205 part.cpp 4ce7e28e1071772921e6292e61a88c905a62d7f6 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 ui/thumbnaillist.h 61601c228512772fd46abc27468523aef562c3fa ui/sidebar.cpp 2474db8c357f6bfd1a9b6bd75091f2eb7b7b7693 ui/pageview.cpp dd4199450672c18ebfa146327d8e9b7eb034ddc8 ui/sidebar.h 036d7788096366d6bab7d7f2a41d55b7a31d303a ui/pageview.h 577b908633bd0778df33d0a15961ab16071a1500 ui/thumbnaillist.cpp 72b557e6624e8229cf1e5e4c5dc69ed77fec54cb Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 19, 2014, 10:48 p.m.) Review request for Okular. Changes --- Copy stdin to temporary file and pass to existing window. The temp file is deleted by the source process as soon as it is opened by the destination process. This behavior seems to be safe on Linux (the file is kept around until it's fully closed), but I'm not sure if it works on other platforms. Also, the file gets a cryptic random name which might not be good. Bugs: 331872 and 332238 http://bugs.kde.org/show_bug.cgi?id=331872 http://bugs.kde.org/show_bug.cgi?id=332238 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs (updated) - ui/pageview.h 577b908633bd0778df33d0a15961ab16071a1500 ui/pageview.cpp dd4199450672c18ebfa146327d8e9b7eb034ddc8 ui/sidebar.h 036d7788096366d6bab7d7f2a41d55b7a31d303a ui/sidebar.cpp 2474db8c357f6bfd1a9b6bd75091f2eb7b7b7693 ui/thumbnaillist.h 61601c228512772fd46abc27468523aef562c3fa ui/thumbnaillist.cpp 72b557e6624e8229cf1e5e4c5dc69ed77fec54cb part.h 010e9de1f2b5c27531a48b943d821ccc3f3f7205 shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 part.cpp 4ce7e28e1071772921e6292e61a88c905a62d7f6 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 18, 2014, 9:03 p.m.) Review request for Okular. Changes --- Add support for opening tabs through drag drop. Support was previously provided only through the PageView and ThumbnailList widgets. Now the entire sidebar widget can accept drops. If tabs are not enabled, or the part is not in native shell mode, DD will function as before. Bugs: 331872 and 332238 http://bugs.kde.org/show_bug.cgi?id=331872 http://bugs.kde.org/show_bug.cgi?id=332238 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs (updated) - ui/thumbnaillist.cpp 72b557e6624e8229cf1e5e4c5dc69ed77fec54cb ui/sidebar.cpp 2474db8c357f6bfd1a9b6bd75091f2eb7b7b7693 ui/thumbnaillist.h 61601c228512772fd46abc27468523aef562c3fa ui/sidebar.h 036d7788096366d6bab7d7f2a41d55b7a31d303a ui/pageview.cpp dd4199450672c18ebfa146327d8e9b7eb034ddc8 ui/pageview.h 577b908633bd0778df33d0a15961ab16071a1500 shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 part.cpp 4ce7e28e1071772921e6292e61a88c905a62d7f6 part.h 010e9de1f2b5c27531a48b943d821ccc3f3f7205 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
On March 17, 2014, 4:29 p.m., Albert Astals Cid wrote: Does this also solve dragdrop as described in https://bugs.kde.org/show_bug.cgi?id=332238 ? No, I have avoided modifying the part/pageview. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/#review53260 --- On March 10, 2014, 5:45 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 10, 2014, 5:45 p.m.) Review request for Okular. Bugs: 331872 http://bugs.kde.org/show_bug.cgi?id=331872 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
On March 17, 2014, 4:29 p.m., Albert Astals Cid wrote: Does this also solve dragdrop as described in https://bugs.kde.org/show_bug.cgi?id=332238 ? Jonathan Doman wrote: No, I have avoided modifying the part/pageview. Albert Astals Cid wrote: Please solve dragdrop too. Okay, that shouldn't be difficult. This patch also doesn't handle reading from stdin. If you have time, I would appreciate your opinion on the best way to handle this, since the data would have to be passed from one process to another. (My best guess is to dump stdin to a temp file and tell the existing process to delete the file when it's closed.) - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/#review53260 --- On March 10, 2014, 5:45 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 10, 2014, 5:45 p.m.) Review request for Okular. Bugs: 331872 http://bugs.kde.org/show_bug.cgi?id=331872 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
On March 17, 2014, 4:29 p.m., Albert Astals Cid wrote: Does this also solve dragdrop as described in https://bugs.kde.org/show_bug.cgi?id=332238 ? Jonathan Doman wrote: No, I have avoided modifying the part/pageview. Albert Astals Cid wrote: Please solve dragdrop too. Jonathan Doman wrote: Okay, that shouldn't be difficult. This patch also doesn't handle reading from stdin. If you have time, I would appreciate your opinion on the best way to handle this, since the data would have to be passed from one process to another. (My best guess is to dump stdin to a temp file and tell the existing process to delete the file when it's closed.) Albert Astals Cid wrote: Caching stdin to another process makes the whole piping thing useless. But sure, i guess there's no other solution, no? An alternative might be forcing stdin to always open a new window. But acrobat can handle putting stdin in an existing window, and I like this behavior. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/#review53260 --- On March 10, 2014, 5:45 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- (Updated March 10, 2014, 5:45 p.m.) Review request for Okular. Bugs: 331872 http://bugs.kde.org/show_bug.cgi?id=331872 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 330991] Unable to select table for a second time (without restarting Okular)
https://bugs.kde.org/show_bug.cgi?id=330991 Jonathan Doman jonathan.do...@gmail.com changed: What|Removed |Added CC||jonathan.do...@gmail.com --- Comment #4 from Jonathan Doman jonathan.do...@gmail.com --- Perhaps AD does not realize you have to press Esc in order to make another selection with the table tool. It differs from the other selection tools and is a bit unintuitive because of this. Taken literally, the steps to reproduce do behave as described. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 116700: Launch documents from external sources in new tabs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116700/ --- Review request for Okular. Bugs: 331872 http://bugs.kde.org/show_bug.cgi?id=331872 Repository: okular Description --- Currently, tabs can only be created by opening files through okular's file dialog. This patch allows external sources (e.g. file browsers) to open documents in tabs of existing windows. Basically, whenever okular is launched it uses D-Bus to search for existing okular instances that are capable of handling its documents. An eligible window will have enough space for all the documents (space depends on whether tabs are allowed or not), and be located on the current desktop. If there are multiple eligible windows, one is chosen at random (the first one in the D-Bus listing). This metric can be changed in the future, but is the simplest for now. To facilitate the D-Bus interaction, two methods were added: canOpenDocs is used to find an eligible window, openDocument is used to actually open a single document. Additionally, tryRaise was modified to work on non-unique windows. I previously thought that okular should behave similar to a web browser, but now I think they are very different. Most people open a web browser and then search and open links completely within it. However, most people will use okular by clicking on files in a file browser. Therefore, I don't think okular should ever open tabs in a window on a different desktop. One shortcoming occurs when one wants to open a document in its own window, but instead gets put into a tab. There is no way to detach a tab, so it's necessary to then open an empty okular instance and use the file dialog. Detaching tabs should be possible, but I guess there are concerns about how to do so efficiently. Diffs - shell/shell.cpp 01a9f1b1dc1992a4aa1c87330940b27b727608e5 shell/shell.h 3b9aae061cd6be0c7a86c885fc06d00d9275bd50 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 Diff: https://git.reviewboard.kde.org/r/116700/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 331872] Open documents in new tab doesn't work
https://bugs.kde.org/show_bug.cgi?id=331872 Jonathan Doman jonathan.do...@gmail.com changed: What|Removed |Added CC||jonathan.do...@gmail.com --- Comment #1 from Jonathan Doman jonathan.do...@gmail.com --- Can you clarify the actions you are taking to reproduce and what is the expected behavior? New tabs will only be created for documents opened through okular's file open dialog. Documents opened through an external file browser or command line will open a new window. If this isn't the behavior you want/expect, you should resubmit this as a feature request and I can make a patch. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 331872] Open documents in new tab doesn't work
https://bugs.kde.org/show_bug.cgi?id=331872 --- Comment #3 from Jonathan Doman jonathan.do...@gmail.com --- Okay, I'm not an official developer but I'll try to implement this and get it reviewed. How would you expect this to behave when there are multiple okular windows already opened? Should it open a new tab in the first window, or any window, or a window determined by some other metric? (I have my own opinion but would like more inputs.) -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 115636: Set tabWidget's documentMode
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115636/ --- (Updated Feb. 13, 2014, 10:25 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Repository: okular Description --- The default tab widget adds a frame around the pageview. This is visually unnecessary and prevents the cursor from hitting the scroll bar when the window is maximized. This simply sets documentMode to remove the frame. Diffs - shell/shell.cpp 822615351a91386c51dad4f4afe67db53c3ec97d Diff: https://git.reviewboard.kde.org/r/115636/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 115636: Set tabWidget's documentMode
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115636/ --- Review request for Okular. Repository: okular Description --- The default tab widget adds a frame around the pageview. This is visually unnecessary and prevents the cursor from hitting the scroll bar when the window is maximized. This simply sets documentMode to remove the frame. Diffs - shell/shell.cpp 822615351a91386c51dad4f4afe67db53c3ec97d Diff: https://git.reviewboard.kde.org/r/115636/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Feb. 8, 2014, 11:16 a.m.) Status -- This change has been marked as submitted. Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Feb. 8, 2014, 3:14 p.m., Christoph Feck wrote: Thanks for this feature, Jonathan! I just tried Okular from master, and noticed that QTabWidget adds a frame around the page view. Is using QTabWidget::setDocumentMode(false) an option, at least for the case, where the tabs are hidden? Christoph Feck wrote: setDocumentMode(true), of course ... :) Yeah, I see that documentMode should definitely be set. Not sure what to do considering this review has already been submitted. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review49254 --- On Feb. 8, 2014, 11:16 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Feb. 8, 2014, 11:16 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 219121] if holding mouse mid button, perform scroll instead zoom
https://bugs.kde.org/show_bug.cgi?id=219121 Jonathan Doman jonathan.do...@gmail.com changed: What|Removed |Added CC||jonathan.do...@gmail.com --- Comment #11 from Jonathan Doman jonathan.do...@gmail.com --- In my experience, this behavior is common in windows applications but in fact very rare on linux. Chromium implements this on windows, but the developers refuse to do so on linux because it's not a linux behavior and because middle click already has a set of confusing precedents (https://code.google.com/p/chromium/issues/detail?id=17689). The X11 evdev EmulateWheel setting should provide a similar functionality without having to implement it in every application. I just tested it and it correctly overrides the zoom in okular. Personally, I don't think there's any need to add this to okular. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Jan. 28, 2014, 12:01 a.m., Albert Astals Cid wrote: Can you confirm that diff --git a/part.rc b/part.rc index 6b1f44e..0b9cee5 100644 --- a/part.rc +++ b/part.rc @@ -84,7 +84,7 @@ Action name=show_leftpanel group=show_merge/ Action name=show_bottombar group=show_merge/ Action name=options_configure_generators group=configure_merge/ -Action name=options_configure/ +Action name=options_configure group=configure_merge/ /Menu Menu name=helptextamp;Help/text Action name=help_about_backend group=about_merge/ fixes the settings menubar jumping issue? Yeah, that seems to keep everything in order. Should I add that to this patch? - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review48432 --- On Jan. 19, 2014, 9:34 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 19, 2014, 9:34 p.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 19, 2014, 9:34 p.m.) Review request for Okular. Changes --- - Disable the closeFindBar action rather than clear its shortcut Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Jan. 15, 2014, 11:33 p.m., Albert Astals Cid wrote: 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 Jonathan Doman wrote: 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. Albert Astals Cid wrote: 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. Sorry, what I meant was that this problem already shows up in any program that embeds multiple okularparts. I'll find a workaround. (As an aside, it seems like the table selection tool interface is not consistent with the other selection tools. Seems like they should all use a click-away-to-clear interface, rather than Esc-to-clear.) - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review47475 --- On Jan. 13, 2014, 12:24 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 13, 2014, 12:24 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Jan. 15, 2014, 11:33 p.m., Albert Astals Cid wrote: 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 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. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review47475 --- On Jan. 13, 2014, 12:24 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 13, 2014, 12:24 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 181 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line181 This check seems a bit weird, i don't even think you need it, but if you want one check there shouldn't you be doing something like activeTab m_tabs.size? But this can't happen, no? Yes, you are right. I was specifically trying to change the code as little as possible, and there was previously a check for m_part which was also weird and unnecessary. On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 206 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line206 should this be m_tabs[activeTab] instead of m_tabs[0]? Maybe. In that section of code it is guaranteed that activeTab==0, since there will never be an empty part when there are other tabs open. I was going to put an assertion in there to make that clear, but I noticed there are no assertions anywhere in the code and thought it might be some kind of policy. On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 256 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line256 space here Are you sure? Most config keys (in this program and others) are camel cased. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review47271 --- On Jan. 11, 2014, 10:32 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 11, 2014, 10:32 p.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 05a0a62265c7e7ed79d719a3f648850f8ef642e5 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 13, 2014, 12:24 a.m.) Review request for Okular. Changes --- - Correct activeTab logic in openUrl() - Add space per request Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 181 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line181 This check seems a bit weird, i don't even think you need it, but if you want one check there shouldn't you be doing something like activeTab m_tabs.size? But this can't happen, no? Jonathan Doman wrote: Yes, you are right. I was specifically trying to change the code as little as possible, and there was previously a check for m_part which was also weird and unnecessary. Albert Astals Cid wrote: Well, the check was checking that the part that we were going to use was really there, since there was just one part, it only checked for the pointer, now you are checking that there is any part and then you access one that may not be there, please change it so it checks for m_tabs.size (Well, I don't think you could ever get in that function if m_part wasn't there.) Anyway, fixed. On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 206 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line206 should this be m_tabs[activeTab] instead of m_tabs[0]? Jonathan Doman wrote: Maybe. In that section of code it is guaranteed that activeTab==0, since there will never be an empty part when there are other tabs open. I was going to put an assertion in there to make that clear, but I noticed there are no assertions anywhere in the code and thought it might be some kind of policy. Albert Astals Cid wrote: You sure about that? if a document contains a DocumentAction::Close: action the part will close the document, but the part won't be destroyed, no? Hmmm, or maybe it will actually will. Anyway if it is guaranteed that activeTab is 0, i think it's easier to undertand the code if you actually use activeTab there since it flows better with reading the code. If you want to add an assert that's fine (Q_ASSERT), but also make sure the code does not crash even if the assert is not met for people that run with Q_ASSERT disabled. If there is a way to close the document without going through the shell, then I suppose it is a problem. On Jan. 12, 2014, 9:57 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 256 https://git.reviewboard.kde.org/r/110914/diff/9/?file=233820#file233820line256 space here Jonathan Doman wrote: Are you sure? Most config keys (in this program and others) are camel cased. Albert Astals Cid wrote: Sorry, was unclear, just meant after the comma. Oh, the spacing was intentional. Anyway, fixed. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review47271 --- On Jan. 13, 2014, 12:24 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 13, 2014, 12:24 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 88e2c41059344dbd11d4c94f99c63a3f5bc8c99b shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 11, 2014, 10:32 p.m.) Review request for Okular. Changes --- - Use KStandardShortcut functions rather than enum - Fixed close tab logic - Check result of closeUrl before closing tabs - Switch to modified tab when save dialog is shown on exit Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 05a0a62265c7e7ed79d719a3f648850f8ef642e5 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 9, 2014, 12:15 a.m.) Review request for Okular. Changes --- - Add openInTab setting - Add mimeTypeChanged(KMimeType::Ptr) signal to Okular::Part, which Shell uses to set the tab icon Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 05a0a62265c7e7ed79d719a3f648850f8ef642e5 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 203 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line203 I'd like if you kept the OpenInNewTab/OpenInNewShell in a configuration setting so people were not forced to use tabs if they don't like them Jonathan Doman wrote: Where should the interface for this setting be? Albert Astals Cid wrote: Settings - Configure - General - Program features looks good to me. Waht you think? Jonathan Doman wrote: That configure dialog is for the okular part, while the setting is only applicable to the shell (and not other part users like konqueror). Seems like it should just be a checkbox in the Settings menu since there are no other shell options. Albert Astals Cid wrote: You're right, but on the other hand the shell/part split is something technical, the user should not care much. You're also right in which we should not show the option in other shells like konqueror. Still I don't think a settings checkbox menu is the best option. My idea of the ideal scenario would be the part providing an interface as the ones in interfaces/ where you can add your own configuration stuff to the dialog. But for the sake of not having this review last forever, ok, let's do the checkbox in the settings menu and I'll see if I can implement my idea. I agree with everything you said. For now, I have added a checkbox next to the FullScreen setting. It opens documents in a new tab by default and can be unchecked to use new windows instead. On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 551 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line551 I don't understand the comment Jonathan Doman wrote: Using sender() is generally considered a Bad Practice, but it's simpler than using a signal mapper (greatly so if we will ever allow the tabs to be moved/reordered). The comment is there for others who may be inclined to copy/paste the code. Albert Astals Cid wrote: Personally i don't feel we need that warning, but ok. Comment was removed when refactoring On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 589 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line589 This doesn't seem the best thing, given that okularpart knows the mimetype once it opens the file, i'd prefer a signal beign emmited instead another KMimeType call. Jonathan Doman wrote: Is KMimeType expensive? Are you suggesting that I add a new signal to Okular::Part? Albert Astals Cid wrote: It is not expensive, but we're doing quite a few tricky things in the mimetype determination side besides, and that would probably like when you open stuff form the stdin piping, so not doing the same twice seems like a good idea. Yes, i'm suggesting a new signal or add a paramater to an existing one if you find something suitable. Okular::Part now emits a signal after calculating the mimetype in openFile(). This is used by the Shell to set the tab icon. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review46407 --- On Jan. 9, 2014, 12:15 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 9, 2014, 12:15 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 203 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line203 I'd like if you kept the OpenInNewTab/OpenInNewShell in a configuration setting so people were not forced to use tabs if they don't like them Jonathan Doman wrote: Where should the interface for this setting be? Albert Astals Cid wrote: Settings - Configure - General - Program features looks good to me. Waht you think? That configure dialog is for the okular part, while the setting is only applicable to the shell (and not other part users like konqueror). Seems like it should just be a checkbox in the Settings menu since there are no other shell options. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review46407 --- On Jan. 3, 2014, 2:24 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 3, 2014, 2:24 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 3, 2014, 2:24 a.m.) Review request for Okular. Changes --- Use KTabWidget and fix some other issues Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 203 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line203 I'd like if you kept the OpenInNewTab/OpenInNewShell in a configuration setting so people were not forced to use tabs if they don't like them Where should the interface for this setting be? On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 551 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line551 I don't understand the comment Using sender() is generally considered a Bad Practice, but it's simpler than using a signal mapper (greatly so if we will ever allow the tabs to be moved/reordered). The comment is there for others who may be inclined to copy/paste the code. On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 589 https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line589 This doesn't seem the best thing, given that okularpart knows the mimetype once it opens the file, i'd prefer a signal beign emmited instead another KMimeType call. Is KMimeType expensive? Are you suggesting that I add a new signal to Okular::Part? - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review46407 --- On Jan. 3, 2014, 2:24 a.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/ --- (Updated Jan. 3, 2014, 2:24 a.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 Diff: https://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Dec. 16, 2013, 6:58 a.m.) Review request for Okular. Changes --- Okay, I simplified the tab related features. No DBus changes, no cmdline interface changes (multiple arguments opens multiple windows), no close warnings. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs (updated) - shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Oct. 1, 2013, 6:57 p.m., Fabio D'Urso wrote: Hi! :) I have neither tested nor read the whole patch in depth, I've only had a look at your description and made a few tests. We had a discussion at the okular BOF this summer, and we decided that it's better to avoid having multiple open copies of the same document, because this creates consistency issues if you are annotating (while saving, the two instances will overwrite each other's changes). In the light of this, I'm not sure Duplicate tab makes sense the way it is now. In my opinion, a better approach would be to tie multiple PageViews to the same Document. However, as the Document class currently assumes that there is only one PageView, this requires significant changes, so we had better leave this feature for a different patch. I see a lighter issue with the way Detach tab works: if you create a new process you will 1) loose all cached pixmaps, 2) cause the Part::queryClose() message to show up if there are unsaved annotations. (If you want to reproduce this issue annotate a PDF, click save as, open the file you saved, detach its tab). Both Duplicate tab and Detach tab fail at handling input from - (standard input) cat somePdfFile.pdf | okular - because, by definition, you can't read it twice. About the issue with moving menu items, I see that all you need to do is to add the confirm_tabs_close checkbox, right? Maybe you can just add it to the configuration dialog instead or drop it at all (for example the warnings from document.cpp warnLimitedAnnotSupport() cannot be reactivated). So should I remove the duplicate tab feature? It's possible in the master code to open the same document in two windows, and it would still be possible in this code to manually open the same document to get it in another tab. Is that going to be blocked in the future? If not, I don't see how duplication causes any additional problems. Do you agree with the logic to open new documents in the MRU existing window? No one has given their opinion on this yet. If not, then it's easy to keep documents in the same process and I'll do that. I'll take a look at the other stuff and upload a new diff when I get some time. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review41071 --- On Aug. 23, 2013, 8:06 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 23, 2013, 8:06 p.m.) Review request for Okular. Bugs: 155515 http://bugs.kde.org/show_bug.cgi?id=155515 Repository: okular Description --- 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 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular
[Okular-devel] [okular] [Bug 155515] Switch to tabbed interface
https://bugs.kde.org/show_bug.cgi?id=155515 --- Comment #67 from Jonathan Doman jonathan.do...@gmail.com --- The code is ready to merge from my perspective, but we are still working through the review stage. Unfortunately, we have not really gotten into any discussion about the UI design decisions yet, and I suspect the maintainers will want me to change a lot of things. So, at this rate it may be a couple months yet before it's merged. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: part.cpp, line 838 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183074#file183074line838 This looks a bit weird, you never initialize nor use m_dbusObjectName for anything other than for calling unregisterObject, is this something stale from old changes? Jonathan Doman wrote: I accidentally removed the initialization in the last update. I believe it's necessary so that closed tabs don't show up in DBus. Albert Astals Cid wrote: Are you sure about that? I expected the deletion of an object to cause it being unregistered from the bus, isn't that the case? It seems to work here in a simple test i made Jonathan Doman wrote: This may be another bug/feature with qdbusviewer (it crashes itself and okular if the parts aren't unregistered). How do you test dbus stuff? `dbus-send --dest=org.kde.okular-pid --print-reply / org.freedesktop.DBus.Introspectable.Introspect` shows closed parts if they aren't unregistered. Albert Astals Cid wrote: I'm just using qdbus command line tool with the current okular code. Open two files in the same okular binary and i have $ qdbus org.kde.okular-10275 /okular /okular2 Close one of the windows and $ qdbus org.kde.okular-10275 /okular I can reproduce your example with the current code. However, with my code qdbus crashes itself and okular if the parts are not unregistered. So I'll try to figure out why that is. On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: shell/main.cpp, line 112 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183075#file183075line112 This looks like a separate feature than the tabs feature, maybe makes sense to split it to a different review to make it easier to review? Jonathan Doman wrote: By default, documents try to open as a new tab in an existing window. So without --new, the Detach Tab feature wouldn't work. Unless you want to change the default behavior, --new has to come in at the same time. Albert Astals Cid wrote: Oh, didn't see that you are spawning a whole new process. Why are you doing that? We can have multiple toplevel windows in a given process, no? Jonathan Doman wrote: I think it makes more sense to have each window in its own process, especially so that each window has its own DBus service. I want the command `okular file.pdf` to open file.pdf in the most recently activated window. Either we force all windows into one process and keep static state about the Shell activation times, or put each Shell into its own process and query activation times over DBus. It would be unnecessarily complicated to allow a mix, and I think the latter option is simpler to implement from the current state of things. Albert Astals Cid wrote: Well, we do have multiple windows in multiple processes now. Don't see why you need to change that to support tabs. If you want to propose this change, fine, but i can't see how it's necessary for the tabs feature. Before discussing this, I think it would be best to decide how the interface should operate. If you agree with my choice of selecting the last activated window to open a document in, then I can defend the choice to put each windows in its own process (the code is much simpler). But if you propose a different interface, then this discussion may become irrelevant. Is there any problem with one-window-per-process that I don't see? The code difference is trivial from my point of view. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review38086 --- On Aug. 23, 2013, 8:06 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 23, 2013, 8:06 p.m.) Review request for Okular. Description --- 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
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: part.cpp, line 838 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183074#file183074line838 This looks a bit weird, you never initialize nor use m_dbusObjectName for anything other than for calling unregisterObject, is this something stale from old changes? Jonathan Doman wrote: I accidentally removed the initialization in the last update. I believe it's necessary so that closed tabs don't show up in DBus. Albert Astals Cid wrote: Are you sure about that? I expected the deletion of an object to cause it being unregistered from the bus, isn't that the case? It seems to work here in a simple test i made This may be another bug/feature with qdbusviewer (it crashes itself and okular if the parts aren't unregistered). How do you test dbus stuff? `dbus-send --dest=org.kde.okular-pid --print-reply / org.freedesktop.DBus.Introspectable.Introspect` shows closed parts if they aren't unregistered. On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: shell/main.cpp, line 112 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183075#file183075line112 This looks like a separate feature than the tabs feature, maybe makes sense to split it to a different review to make it easier to review? Jonathan Doman wrote: By default, documents try to open as a new tab in an existing window. So without --new, the Detach Tab feature wouldn't work. Unless you want to change the default behavior, --new has to come in at the same time. Albert Astals Cid wrote: Oh, didn't see that you are spawning a whole new process. Why are you doing that? We can have multiple toplevel windows in a given process, no? I think it makes more sense to have each window in its own process, especially so that each window has its own DBus service. I want the command `okular file.pdf` to open file.pdf in the most recently activated window. Either we force all windows into one process and keep static state about the Shell activation times, or put each Shell into its own process and query activation times over DBus. It would be unnecessarily complicated to allow a mix, and I think the latter option is simpler to implement from the current state of things. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review38086 --- On Aug. 18, 2013, 11:37 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 18, 2013, 11:37 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 23, 2013, 8:06 p.m.) Review request for Okular. Changes --- Fixed pointer check coding style Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 20, 2013, 9:40 p.m., Albert Astals Cid wrote: shell/shell.cpp, line 669 http://git.reviewboard.kde.org/r/110914/diff/4/?file=183348#file183348line669 one more nitpick about style, when comparing pointers just do if (part) instead of if (part != 0) and if (!part) instead of if (part == 0) Is there a coding guidelines document that you follow? It seems many of my preferences are contrary to yours. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review38244 --- On Aug. 23, 2013, 8:06 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 23, 2013, 8:06 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: shell/main.cpp, line 53 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183075#file183075line53 Just commenting here, but please try to review all your code. It's good if you can try to make all the variables that never change (like services) const, this way a second person that reads over the code, does not have to struggle finding if that may change or not. Yes i know we don't apply that over all the okular code, but we're trying to add good practices :-) Do you want everything as const as possible? For example, there are places I can do const Type* const but I think Type* or const Type* is cleaner. How about function parameters (int vs const int)? - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review38086 --- On Aug. 17, 2013, 3:06 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 17, 2013, 3:06 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 18, 2013, 11:37 p.m.) Review request for Okular. Changes --- Initialize m_dbusObjectName. Style fixes for NULL/0 and constness. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: part.cpp, line 838 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183074#file183074line838 This looks a bit weird, you never initialize nor use m_dbusObjectName for anything other than for calling unregisterObject, is this something stale from old changes? I accidentally removed the initialization in the last update. I believe it's necessary so that closed tabs don't show up in DBus. On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: shell/main.cpp, line 23 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183075#file183075line23 Why do you need this? I don't. Deleted it. On Aug. 18, 2013, 8:41 p.m., Albert Astals Cid wrote: shell/main.cpp, line 112 http://git.reviewboard.kde.org/r/110914/diff/3/?file=183075#file183075line112 This looks like a separate feature than the tabs feature, maybe makes sense to split it to a different review to make it easier to review? By default, documents try to open as a new tab in an existing window. So without --new, the Detach Tab feature wouldn't work. Unless you want to change the default behavior, --new has to come in at the same time. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review38086 --- On Aug. 18, 2013, 11:37 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 18, 2013, 11:37 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 17, 2013, 3:06 p.m.) Review request for Okular. Changes --- Reverted changes to DBus names Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 14, 2013, 10:29 p.m., Albert Astals Cid wrote: When I started looking at dbus, I couldn't get anything to work. Whenever I tried to run any org.kde.okular method in qdbusviewer, it would say unable to find method or something similar. So I thought the problem might be related to the fact that two objects were each exporting an interface with the same name (org.kde.okular) but with different methods. When I changed the interface names, everything seemed to start working. I've never used dbus before this, so my understanding could be off. But basically, I had to do this for it to work on my computer. I'll go back again to see if it works without changing the names. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review37814 --- On Aug. 3, 2013, 10:03 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 3, 2013, 10:03 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 part.h 4b3aafdb637080ae81eb0e45742f53a34738984d shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 14, 2013, 10:29 p.m., Albert Astals Cid wrote: shell/shell.h, line 49 http://git.reviewboard.kde.org/r/110914/diff/2/?file=176095#file176095line49 Why are you changing the dbus names? This will break whatever scripts people where using. It seems my experiences were caused by a bug/feature in qdbusviewer (https://bugreports.qt-project.org/browse/QTBUG-6943). So, while technically it's okay to keep the interface names how they are, it makes more sense to me that they be different. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review37814 --- On Aug. 3, 2013, 10:03 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 3, 2013, 10:03 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 part.h 4b3aafdb637080ae81eb0e45742f53a34738984d shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 3, 2013, 10:03 p.m.) Review request for Okular. Changes --- Removed all the simple reformatting changes I could find Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs (updated) - part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 part.h 4b3aafdb637080ae81eb0e45742f53a34738984d shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On June 9, 2013, 10:29 p.m., Albert Astals Cid wrote: part.cpp, line 305 http://git.reviewboard.kde.org/r/110914/diff/1/?file=149252#file149252line305 Please avoid reformating source code, makes it harder to review Albert Astals Cid wrote: Hello? Is anyone there? Sorry, I thought you meant nobody was going to look at it for six months. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review34000 --- On Aug. 3, 2013, 10:03 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 3, 2013, 10:03 p.m.) Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 part.h 4b3aafdb637080ae81eb0e45742f53a34738984d shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- Review request for Okular. Description --- 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 This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 10b9e6fc394e6804d78e99611a15e566198c0649 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 155515] Switch to tabbed interface
https://bugs.kde.org/show_bug.cgi?id=155515 --- Comment #62 from Jonathan Doman jonathan.do...@gmail.com --- Thank you for your feedback. I had not considered the case presented in #58. I will take these comments into consideration and open a review request when I think the tabs behave reasonably. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 155515] Switch to tabbed interface
https://bugs.kde.org/show_bug.cgi?id=155515 Jonathan Doman jonathan.do...@gmail.com changed: What|Removed |Added CC||jonathan.do...@gmail.com --- Comment #57 from Jonathan Doman jonathan.do...@gmail.com --- I have started work on a tabbed interface, currently hosted here: https://github.com/jrmrjnck/okular-tabbed. Devs: if you are interested in using my code, I will be happy to continue working on it and go through whatever review process you have. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
