oasegips / sigil

Automatically exported from code.google.com/p/sigil
GNU General Public License v3.0
0 stars 0 forks source link

Sigil - mixing up fonts&CSS-styles while working in WYSIWYG mode ... #2423

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello all,

(1st, I really read the guide how to post issues - believe me I am from big 
corporate - we also have standards for messages and issues - however my issue 
doesn't fit into your message pattern -> so allow me use for once different 
format)

1st of all - Sigil is great editor. After evaluating many others we decided to 
use Sigil for our 1st eBook which was successfully published for both iTunes 
and Amazon KDP. 

However after this one year of >real experience< with Sigil I decided to report 
one (to me the biggest) issue with Sigil - it's the fact that WYSIWYG editor 
doesn't work well for very simple editing tasks (type-in text, copy&paste text, 
etc ...). Everything seems to work fine -> but if you edit for a while using 
WYSIWYG editor and then switch to HTML - there is a mess of <span>, <font> tags 
and CSS styles ... Also if you switch  back to WYSIWYG - sometimes you see the 
text you pasted-in or typed-in in slightly different font, but sometimes it is 
very hard to see the difference. Then you export the ePub to Amazon KDP i.e. 
and there the difference between the various styles is much more visible. 

So long story short - using WYSIWYG editor mixes up the fonts and styles and 
adds unnecessary <span> tags which creates a hell which you end up cleaning up 
manually in HTML code ... :-(

Believe me I tried to find a reproduction pattern - but it's not easy. To 
reproduce - just play with the Sigil .. load a ePub with some CSS styles ... 
start editing it in WYSIWYG mode (use copy&paste form other parts of the 
document AND ALSO from other applications running in your OS ... )

You will very quickly realize that the logic in WYSIWYG mode is not good - 
<span> and <font> tags are added where it is not necessary - sometimes even 
CSS-styles used are decided based on who knows which logic ... 

My biggest suspect is the "paste from clipboard" function -> it probably takes 
the clipboard data including formatting metadata -> then it tries to fit-in the 
formatting metadata into one of existing CSS styles or it just adds pure <font> 
tag with the formatting info ... the logic is not clear to me ... 

IMHO the functionality should be in 99% of use-cases just to try to extract the 
text from the clipboard (and ignore formatting there) and paste it into 
existing html tag not trying to change the format/style. For the rest of the 
cases - this 1% - when you want to copy the formatting as well - this is tricky 
- and ideally Sigil should try to match the format from clipboard against 
format of existing CSS styles.
Thus I would suggest following behavior for paste function:

-> no formatting data available -> just paste it as it was typed in manually

-> formatting data available -> ask end-user whether he want's to:
   a) paste without format (default)
   b) paste with best matching CSS-style (and show which ones were identified as best matching and give option to change it to any existing CSS style)
   c) paste with current formatting info (that would end up using <font> tag with OS native fonts - as it already happens nowadays ... (not sure who would use this option if writing ePub to be published across various platforms)

Optionally you can think of having the default choice being configured in Sigil 
preferences and/or having several paste functions in the Edit menu (Paste Text 
Only, Paste with Macthing CSS, Paste as is ...) ... but think of usability - my 
experience with editor is that you do not want to paste the formatting info in 
99% of the cases while your work on the eBook ... 

!!! BUT !!! -> I have also seen strange <span> tags to be added into the html 
code even I just clicked inside the text and start just typing (or maybe 
selected text and typing/replacing it), or changed the format to bold (Ctrl+B) 
i.e. but no paste from clipboard ... however this happens only sometimes ... 
and for me it's hard to reproduce it ... but it is there as well.

I have seen those issues above on 2 platforms - MacOSX and Windows ... I tried 
earlier versions of Sigil 0.6.x and also most recent 0.7.2 and 0.7.3 -> the 
same problem still exists.

To me - the correction of this behavior would be major quick-win for Sigil's 
productivity and thus it's value!! See also this issue - 
http://code.google.com/p/sigil/issues/detail?id=2019 - it is requesting better 
CSS styles cleanup logic. I agree it can be achieved using the suggested regex 
serach&replace and additional manual functions - however this is not the point. 
In 1st place the wrong WYSIWYG editor behavior is causing the whole document to 
be filled with unnecessary formatting tags.

Thanx and best regards, Richard.

PS: I'll try to look into source code whether I'll succeed to identify the 
WYSIWYG paste logic improvements ... no promises :-)

Original issue reported on code.google.com by cal...@gmail.com on 23 Oct 2013 at 11:02

GoogleCodeExporter commented 9 years ago
Okay - so I did some debugging in Latest source code 0.7.3 git clone (from 
Oct-24-2013) and here are some of my findings:

---> Keyboard-Shortcut Paste function (CTRL+V or SHIFT+Insert) goes around the 
Sigil application and is directly handled by the QT5 library I guess ... this 
is causing the clipboard HTML formatting data are pasted to the eBook HTML 
according to what you have inserted from another application -> thus causing 
mess in your eBook HTML code

      => quick workaround : redirect the CTRL+V and SHIFT-Insert shortcuts on app level and direct them to the same functionality which Sigil uses to Paste to the BookView (menu entry Edit -> Paste, Edit -> Paste from Clipoard History) - see below.

      => THIS WILL ALIGN THE PASTE FUNCTION TO BEHAVE CONSISTENTLY IN SIGIL. IN CURRENT CODCE THIS IS NOT DONE AND THIS ALLOWS UNCONTROLLED FORMATTING DATA TO SNEAK INTO SIGIL GENERATED HTML

      => BUG: In addition - because the keyboard shortcuts are not handled by Sigil -> Undo functionality doesn't work with them

---> Menu entry Edit -> Paste, Edit -> Paste from Clipboard History-> is 
processed by coding:

---> For Code View (HTML) - file CodeViewEditor.cpp (line 1973)

void CodeViewEditor::PasteText(const QString &text)
{
    InsertText(text);
}

function InsertText:

void CodeViewEditor::InsertText(const QString &text)
{
    QTextCursor c = textCursor();
    c.insertText(text);
    setTextCursor(c);
}

---> For Book View (WYSIWYG) - file BookViewEditor.Cpp (line 626)

void BookViewEditor::paste()
{
    page()->triggerAction(QWebPage::Paste);
}

================================ DISCUSSION ================================

Even if the Paste function is processed by Sigil code. There is stile some 
nasty HTML code being pasted, in my example I tried I got this included as 
additional formatting info: <span style="font-family: 'Times New Roman', serif; 
font-size: 12pt;">

Which was much better comparing what I got if I used same clipoard data and 
used CTRL+V Paste function.

Question 1 - what would QT5 action - QWebPage::PasteAndMatchStyle - do if used 
??? Is it trying to mathc best existing CSS style ???

Question 2 - if we use the pure "Text" Paste from the CodeViewEditor - we will 
loose formatting data I guess but maybe the whole user experienece would be 
better as we do not want to primarily incldue external formatting information 
into our eBook ... 

Will continue looking into it ... 

Original comment by cal...@gmail.com on 25 Oct 2013 at 11:09

GoogleCodeExporter commented 9 years ago
Ok ... the old function 

void BookViewEditor::paste()
{
    page()->triggerAction(QWebPage::Paste);
}

if replaced by following code:

class QMessageBoxResize: public QMessageBox
{
    public:
        QMessageBoxResize() {
            setMouseTracking(true);
            setSizeGripEnabled(true);
        }

    private:
        virtual bool event(QEvent *e) {
            bool res = QMessageBox::event(e);
            switch (e->type()) {
                case QEvent::MouseMove:
                case QEvent::MouseButtonPress:
                setMaximumSize(QWIDGETSIZE_MAX, QWIDGETSIZE_MAX);
                if (QWidget *textEdit =  findChild<QTextEdit *>()) {
                    textEdit->setMaximumHeight(QWIDGETSIZE_MAX);
                }
            }
            return res;
        }
};

void BookViewEditor::paste()
{
    // original clipboard format including foreign formatting info:
    // page()->triggerAction(QWebPage::Paste);

    // text only (ignore foreign formatting info) -> better formatting for eBook editing
    // page()->triggerAction(QWebPage::PasteAndMatchStyle);

    QClipboard *clipboard = QApplication::clipboard();

    if ( clipboard->mimeData()->hasHtml() )
    { 

        // allow resize function to see whole HTML in the Detailed text window
        // doesn't work:
        // QMessageBox msgBox;
        // msgBox.setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
        // msgBox.setSizeGripEnabled(true);
        // so using QMessageBoxResize override:

        QMessageBoxResize msgBox;
        msgBox.setIcon(QMessageBox::Question);
        msgBox.setText("Clipboard contains HTML formatting.");
        msgBox.setInformativeText("Do you want to paste clipboard data as plain text instead?");
        msgBox.setStandardButtons(QMessageBox::Yes| QMessageBox::No| QMessageBox::Cancel);
        msgBox.setDefaultButton(QMessageBox::Yes);

        // populate the detailed text window - by HTML not by the text
        msgBox.setDetailedText(clipboard->mimeData()->html());

        // show message box
        int ret = msgBox.exec();

        switch (ret) {
          case QMessageBox::Yes:
              // Yes was clicked
              page()->triggerAction(QWebPage::PasteAndMatchStyle);
              break;
          case QMessageBox::No:
              // No was clicked
              page()->triggerAction(QWebPage::Paste);
              break;
          case QMessageBox::Cancel:
              // Cancel was clicked - do nothing
              break;
          default:
              // should never be reached
              break;
        }

    } else {
        page()->triggerAction(QWebPage::Paste);
    }

}

allows to change the paste function behavior override, it works as this:

If HTML formatted code is contained in the clipboard - a newly added message 
box is shown to user allowing the end user to decide whether he wants to either 
paste only text without the formatting data - default choice - (invoking 
QWebPage::PasteAndMatchStyle) or whether end user want's to continue with 
pasting the original HTML formatted code. User has also option to use button 
Show details to see the HTML content of the clipboard in resize-able MessageBox.

In addition to allow CTLR+V, Command+V and SHIFT+Insert key shortcuts to work 
same way as the conext menu and main menu entries - Paste - additional changes 
are needed to be done to BookViewEditor.h and BookViewEditor.cpp.

Original comment by cal...@gmail.com on 29 Oct 2013 at 4:58

GoogleCodeExporter commented 9 years ago
Have created user-none pull request to include the changes into the main Sigil 
repository.

https://github.com/user-none/Sigil/pull/8

Original comment by cal...@gmail.com on 29 Oct 2013 at 5:13

GoogleCodeExporter commented 9 years ago

Original comment by john@nachtimwald.com on 3 Nov 2013 at 2:26