View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0016180 | Scribus | Story Editor / Text Frames | public | 2020-07-20 10:27 | 2020-07-21 08:03 |
| Reporter | ale | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | new | Resolution | open | ||
| Product Version | 1.5.6.svn | ||||
| Summary | 0016180: PATCH: marks: fail early if it's a master page (and no variable text) | ||||
| Description | don't wait for the dialog to almost show up... | ||||
| Tags | No tags attached. | ||||
| Patch | Yes | ||||
|
|
mark-master-page.diff (852 bytes)
diff --git a/scribus/scribus.cpp b/scribus/scribus.cpp
index d06e7ba8a..de5d92d0a 100644
--- a/scribus/scribus.cpp
+++ b/scribus/scribus.cpp
@@ -9616,6 +9616,9 @@ void ScribusMainWindow::insertMark(MarkType mType)
{
if (!HaveDoc)
return;
+ if (doc->masterPageMode() && (mType != MARKVariableTextType))
+ // only variable text can go on master pages
+ return;
if (doc->m_Selection->count() != 1)
return;
if (doc->appMode != modeEdit)
@@ -9759,10 +9762,6 @@ void ScribusMainWindow::slotInsertMarkNote()
bool ScribusMainWindow::insertMarkDialog(PageItem_TextFrame* currItem, MarkType mrkType, ScItemsState* &is)
{
- if (doc->masterPageMode() && (mrkType != MARKVariableTextType))
- //avoid inserting in master pages other marks than Variable Text
- return false;
-
QScopedPointer<MarkInsert> insertMDialog;
switch (mrkType)
{
|
|
|
The insertMarkDialog() function itself must return in these cases, so I do not see why these checks should be removed from this function. |
|
|
because you do not even need to call the insertMarkDialog() in those cases. except if i did not understand something. then it's really necessary to document the corner cases! as far as i understand it, if the user is on a master page and wants to insert a mark that is not a variable text, she just can't. why bother to get that far? and there is only one function calling insertMarkDialog in the whole scribus code base. imo, this is a good example of code that does not respect the separation of concerns. |
|
|
>> because you do not even need to call the insertMarkDialog() in those cases. Then that's one more reason for those checks to stay. Nothing prevents to add these checks to insertMark() tho. |
|
|
it's definitively too late to check for it there, but if you think that the function could be used in a different context checking that the text frame is not on a master page makes sense. /me would wish that the pre/post conditions camp had won the race... if the refactoring of the marks is accepted this ticket can be closed and i will take into consideration that the text frame must be checked again. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-07-20 10:27 | ale | New Issue | |
| 2020-07-20 10:27 | ale | File Added: mark-master-page.diff | |
| 2020-07-20 10:39 | ale | Summary | marks: fail early if it's a master page (and no variable text) => PATCH: marks: fail early if it's a master page (and no variable text) |
| 2020-07-20 10:39 | ale | Patch | No => Yes |
| 2020-07-20 13:05 | jghali | Note Added: 0047857 | |
| 2020-07-20 13:27 | ale | Note Added: 0047859 | |
| 2020-07-20 21:35 | jghali | Note Added: 0047861 | |
| 2020-07-20 21:39 | jghali | Note Edited: 0047861 | |
| 2020-07-21 08:02 | ale | Note Added: 0047864 | |
| 2020-07-21 08:03 | ale | Note Edited: 0047864 |
