case change destroys links

AlanHK · 05-10-2016, 11:55 AM

I had a TOC with the text in all caps, and wanted to make it title case.

Original text:

Code:

 <div class="sgc-toc-level-1">
    <a href="../Text/chap-17.html#u2670f1c8-139b-4106-95f7-1eb90b10f75f">DISTINGUISHED PERSONS VISIT A TEMPLE PAVILION</a>
  </div>

  <div class="sgc-toc-level-1">
    <a href="../Text/chap-18.html#u28aca12c-1439-40d3-8bf3-796169f2fe9f">A BUDDHIST ABBOT SURPRISES A GUEST</a>
  </div>

In book view, I selected one entry and converted it, got:

Code:

  <div class="sgc-toc-level-1">
    <a href="../Text/chap-17.html#u2670f1c8-139b-4106-95f7-1eb90b10f75f">Distinguished Persons Visit A Temple Pavilion</a>
  </div>

Which was what I wanted, so I selected all the entries and applied the case change, and it looked fine. Then I looked at the code:

Code:

<div class="sgc-toc-level-1">
      <font color="#0000ee">Distinguished Persons Visit A Temple Pavilion</font>
    </div>

    <div class="sgc-toc-level-1">
      <font color="#0000ee">A Buddhist Abbot Surprises A Guest</font>
    </div>

The links are gone and replaced with a blue colouring.
And "undo" does not undo this. So I aborted and reopened.

In code view case change across a tag boundary just does nothing at all, not helpful, but at least not destructive like the above.

I know the work around: just do one at a time.

I don't know if it can be described as a bug, but to me this doesn't seem a good way for change case to work.

KevinH · 05-10-2016, 12:15 PM

In general, doing anything that spans across tags in BookView is never recommended. You simply can not control beginning and end positions with enough precision based on just a BookView cursor position or highlight. Even something as easy as adding split markers when in BookView can have unexpected results.

So when doing something that crosses tag elements, I strongly recommend you work in CodeView.

As to your particular bug, will you please show a larger snippet of the code so that we can see where exactly the font colour tag came from. It must be there close-by and your highlight must have touched upon it in some way.

Better yet ... please run a obfuscator or otherwise change just the text in that html file (if it is copyrighted and not public) keeping its exact tag structure and then zipping it up and posting it here, so that we can use it to try and reproduce what you are seeing and why it happens.

Thanks,

KevinH

AlanHK · 05-10-2016, 12:55 PM

Quote:

Originally Posted by KevinH

In general, doing anything that spans across tags in BookView is never recommended. You simply can not control beginning and end positions with enough precision based on just a BookView cursor position or highlight. Even something as easy as adding split markers when in BookView can have unexpected results.

I do little if any editing in book view, since it often creates a swarm of unwanted tags, nbsp, etc. I tried it here only because it seemed to save time, till I realised what it was really doing.

Quote:

Originally Posted by KevinH

As to your particular bug, will you please show a larger snippet of the code so that we can see where exactly the font colour tag came from. It must be there close-by and your highlight must have touched upon it in some way.

Better yet ... please run a obfuscator or otherwise change just the text in that html file (if it is copyrighted and not public) keeping its exact tag structure and then zipping it up and posting it here, so that we can use it to try and reproduce what you are seeing and why it happens.

Sorry, the original "ALL CAPS" version is gone now, and I've done a fair bit of work on the file since.
There were no font or color tags at all in the original file.
It was generated using Sigil's "Create HTML Table of Contents".

I will see if I can recreate it and upload later.

AlanHK · 05-10-2016, 02:04 PM

Here's a complete TOC file with 2 links:

Code:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
  "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

<html xmlns="http://www.w3.org/1999/xhtml">
<head>
  <title>Contents</title>
  <link href="../Styles/page_styles.css" type="text/css" rel="stylesheet"/>
  <link href="../Styles/stylesheet.css" type="text/css" rel="stylesheet"/>
</head>

<body>
  <div class="sgc-toc-title">
    Contents
  </div>

  <div class="sgc-toc-level-1">
    <a href="../Text/chap-01.html">First Chapter</a>
  </div>

  <div class="sgc-toc-level-1">
    <a href="../Text/chap-02.html">Second Chapter</a>
  </div>
</body>
</html>

Select both links and apply uppercase button in book view:

Code:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
  "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

<html xmlns="http://www.w3.org/1999/xhtml">
<head>
  <title>Contents</title>
  <link href="../Styles/page_styles.css" type="text/css" rel="stylesheet"/>
  <link href="../Styles/stylesheet.css" type="text/css" rel="stylesheet"/>
</head>

<body>
  <div class="sgc-toc-title">
    Contents
  </div>

  <div class="sgc-toc-level-1">
    <div class="sgc-toc-level-1">
      <font color="#0000ee">FIRST CHAPTER</font>
    </div>

    <div class="sgc-toc-level-1">
      <font color="#0000ee">SECOND CHAPTER</font>
    </div>
  </div>
</body>
</html>

Links are gone, replaced by font colour tags.

KevinH · 05-10-2016, 02:38 PM

Huh ...
Where are the font color tags coming from? They must be internally used inside of BookView as they are not in your source.

Wow! That is a strange one.

I'll see if I can track down what BookView is doing and get it fixed. Thanks for your bug report.

KevinH

KevinH · 05-10-2016, 03:13 PM

Okay, what is happening in BookView is that when you highlight more than one piece of text you are thereby highlighting all of the div tags that hold the anchor tags.

The routine then asks for the text equivalent of the contents of the div tags and then uppercases that text content. This removes all other nested tags including the anchor tags as they are simply replaced with the text they contain.

My tests do not show any added font tags at all but they may come from the stylesheets you link to that do not exist in my tests.

When you only selct one pieces of text you get just the anchor tag and its contents are in fact the contents you actually want to uppercase. So it works as expected.

So it is exactly what I described. By highlighting more than one piece of text since you can not select exactly what you want (even though it looks on screen that you are just highlighting the text itself) you are actually selecting a hierarchy of tags and then uppercasing the entire thing.

Which brings me back to exactly why editing in BookView is very dangerous if someone is not aware of exactly what they are highlighting. The visual feedback simply gives that person the wrong impression.

My guess is this same issue exists in BookView using any command/icon that expects text being highlighted and not tags.

Not sure how I can fix this as it is doing what the command was designed to do.

That said, I might be able to detect the use of additional tags inside the div and then abort without changing anything. That seems like the most sensible thing to do.

The more I look at BookView, the more I want to simply remove it and replace it with the ability to copy out of Preview and paste into a small BookView like window.

Thanks for your bug report and test case!

KevinH

KevinH · 05-10-2016, 03:37 PM

Okay after playing some more, whatever you highlight in BV, the code runs a javascript to get the selected text:

QString javascript = "window.getSelection().toString();";
EvaluateJavascript(javascript).toString();

And then runs the case changing code on the text. And then pastes it back over your highlighted selection:

In your test case the font color and underline code are to make the text appear the way it did when it was a link.

But everything you highlight, gets replaced with the case changed text:

Code:

void BookViewEditor::ApplyCaseChangeToSelection(const Utility::Casing &casing)
{
    // This is going to lose any styles that may have been applied within                                                           
    // that selected text.                                                                                                          
    QString selected_text = GetSelectedText();
    qDebug() << "In BookViewEditor" << selected_text;
    QString new_text = Utility::ChangeCase(selected_text, casing);

    if (new_text == selected_text) {
        return;
    }

    // The "proper" way to do a replacement of the text (and allow undo) will                                                       
    // undoubtedly involve reams of javascript. However there is a quick and                                                        
    // dirty alternative, of putting the text on the clipboard and pasting it.                                                      
    // However since this will muck around the with clipboard history we need                                                       
    // to do a little extra work to disconnect and restore things afterwards.                                                       
    emit ClipboardSaveRequest();
    QApplication::clipboard()->setText(new_text);
    paste();
    emit ClipboardRestoreRequest();

    // We will have lost our selection from the paste operation.                                                                    
    for (int i = 0; i < new_text.length(); i++) {
        page()->triggerAction(QWebPage::SelectPreviousChar);
    }
}

The debug print shows exactly what Sigil was provided by the javascript:

Debug: In BookViewEditor "First Chapter
Second Chapter"

So there isn't even an easy way for me to tell that a user has highlighted more than just a piece of text.

I will have to think about how to get the selection in some other way that will let me see if it passes over tag boundaries or not. This one won't be an easy fix or even fixed in the next release.

KevinH

AlanHK · 05-10-2016, 11:03 PM

Quote:

Originally Posted by KevinH

I will have to think about how to get the selection in some other way that will let me see if it passes over tag boundaries or not. This one won't be an easy fix or even fixed in the next release.

In the meantime, since the case buttons are destructive in Bookview, how about make them inoperative? Grey them out so people know it.

But it is a drag trying to text edit in codeview when you have heavily coded text. The only reason I was editing text in Sigil at all is that the text was initially generated in it.

KevinH · 05-11-2016, 12:19 AM

This bug has been around a very long time - since day one of BookView. The issue is people selecting things that cross-over text boundaries unknowingly in BookView.
The buttons do work as advertised if the user properly selects just the text they want to change the case of and does not stretch across tags.
So I hesitate to disable it for BookView but might if I can't come up with something that works. There are just so many many things that can go wrong when editing in BookView, that I think its days are numbered.

KevinH

AlanHK · 05-11-2016, 12:41 AM

It's completely unacceptable for links to be deleted.
Something that destroys code without warning without any "undo" is more a liability than a useful feature.
Better to disable anything that does that in bookview.

What I ideally want is a way to edit the text without being confused by a swarm of code; that leaves the code between the displayed text unchanged. I realise that this becomes problematic if you start cutting and pasting text around from one place to another, joining paragraphs which necessarily restructures the code.

But the special case of changing case, there is no reason to change any formatting code at all. I thought if it was allowed it must be safe.

KevinH · 05-11-2016, 10:19 AM

As I said, if it is truly "unacceptable" to highlight too much and then paste over the top of it just text and then be surprised that it overwrites non-text then do NOT work in BookView.

This can happen in many places and ways in BookView as there is no way to easily undo it since you are effectively changing a DOM tree inside a browser window.

And again as I said in my last post I will look into the best way to handle this case.
KevinH

Turtle91 · 05-11-2016, 10:59 AM

I changed my coding procedures to allow for title case in my TOC and all caps in the chapter. I simply code the title in the chapter how I want it to be displayed in the TOC. Then change the CSS to make the chapter all caps. The auto TOC doesn't take into account the CSS so everything looks fine. eg:

Code:

h2 {text-transform:uppercase}

<h2>Title Case of a Particular Chapter</h2>

How it appears:

TOC: Title Case of a Particular Chapter
Chapter Head: TITLE CASE OF A PARTICULAR CHAPTER

KevinH · 05-11-2016, 12:14 PM

Fixed in master by checking if a user selects more than just text in BookView and if so silently refusing to change case (and thereby do damage).

KevinH

DiapDealer · 05-11-2016, 12:35 PM

Thanks, Kevin. Glad you didn't have to disable it completely.

05-10-2016, 11:55 AM	#1
AlanHK Guru Posts: 681 Karma: 929286 Join Date: Apr 2014 Device: PW-3, iPad, Android phone	case change destroys links I had a TOC with the text in all caps, and wanted to make it title case. Original text: Code: <div class="sgc-toc-level-1"> <a href="../Text/chap-17.html#u2670f1c8-139b-4106-95f7-1eb90b10f75f">DISTINGUISHED PERSONS VISIT A TEMPLE PAVILION</a> </div> <div class="sgc-toc-level-1"> <a href="../Text/chap-18.html#u28aca12c-1439-40d3-8bf3-796169f2fe9f">A BUDDHIST ABBOT SURPRISES A GUEST</a> </div> In book view, I selected one entry and converted it, got: Code: <div class="sgc-toc-level-1"> <a href="../Text/chap-17.html#u2670f1c8-139b-4106-95f7-1eb90b10f75f">Distinguished Persons Visit A Temple Pavilion</a> </div> Which was what I wanted, so I selected all the entries and applied the case change, and it looked fine. Then I looked at the code: Code: <div class="sgc-toc-level-1"> <font color="#0000ee">Distinguished Persons Visit A Temple Pavilion</font> </div> <div class="sgc-toc-level-1"> <font color="#0000ee">A Buddhist Abbot Surprises A Guest</font> </div> The links are gone and replaced with a blue colouring. And "undo" does not undo this. So I aborted and reopened. In code view case change across a tag boundary just does nothing at all, not helpful, but at least not destructive like the above. I know the work around: just do one at a time. I don't know if it can be described as a bug, but to me this doesn't seem a good way for change case to work.

05-10-2016, 12:15 PM	#2
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	In general, doing anything that spans across tags in BookView is never recommended. You simply can not control beginning and end positions with enough precision based on just a BookView cursor position or highlight. Even something as easy as adding split markers when in BookView can have unexpected results. So when doing something that crosses tag elements, I strongly recommend you work in CodeView. As to your particular bug, will you please show a larger snippet of the code so that we can see where exactly the font colour tag came from. It must be there close-by and your highlight must have touched upon it in some way. Better yet ... please run a obfuscator or otherwise change just the text in that html file (if it is copyrighted and not public) keeping its exact tag structure and then zipping it up and posting it here, so that we can use it to try and reproduce what you are seeing and why it happens. Thanks, KevinH Last edited by KevinH; 05-10-2016 at 12:20 PM.

05-10-2016, 03:13 PM	#6
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	Okay, what is happening in BookView is that when you highlight more than one piece of text you are thereby highlighting all of the div tags that hold the anchor tags. The routine then asks for the text equivalent of the contents of the div tags and then uppercases that text content. This removes all other nested tags including the anchor tags as they are simply replaced with the text they contain. My tests do not show any added font tags at all but they may come from the stylesheets you link to that do not exist in my tests. When you only selct one pieces of text you get just the anchor tag and its contents are in fact the contents you actually want to uppercase. So it works as expected. So it is exactly what I described. By highlighting more than one piece of text since you can not select exactly what you want (even though it looks on screen that you are just highlighting the text itself) you are actually selecting a hierarchy of tags and then uppercasing the entire thing. Which brings me back to exactly why editing in BookView is very dangerous if someone is not aware of exactly what they are highlighting. The visual feedback simply gives that person the wrong impression. My guess is this same issue exists in BookView using any command/icon that expects text being highlighted and not tags. Not sure how I can fix this as it is doing what the command was designed to do. That said, I might be able to detect the use of additional tags inside the div and then abort without changing anything. That seems like the most sensible thing to do. The more I look at BookView, the more I want to simply remove it and replace it with the ability to copy out of Preview and paste into a small BookView like window. Thanks for your bug report and test case! KevinH Last edited by KevinH; 05-10-2016 at 03:15 PM.

05-10-2016, 03:37 PM	#7
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	Okay after playing some more, whatever you highlight in BV, the code runs a javascript to get the selected text: QString javascript = "window.getSelection().toString();"; EvaluateJavascript(javascript).toString(); And then runs the case changing code on the text. And then pastes it back over your highlighted selection: In your test case the font color and underline code are to make the text appear the way it did when it was a link. But everything you highlight, gets replaced with the case changed text: Code: void BookViewEditor::ApplyCaseChangeToSelection(const Utility::Casing &casing) { // This is going to lose any styles that may have been applied within // that selected text. QString selected_text = GetSelectedText(); qDebug() << "In BookViewEditor" << selected_text; QString new_text = Utility::ChangeCase(selected_text, casing); if (new_text == selected_text) { return; } // The "proper" way to do a replacement of the text (and allow undo) will // undoubtedly involve reams of javascript. However there is a quick and // dirty alternative, of putting the text on the clipboard and pasting it. // However since this will muck around the with clipboard history we need // to do a little extra work to disconnect and restore things afterwards. emit ClipboardSaveRequest(); QApplication::clipboard()->setText(new_text); paste(); emit ClipboardRestoreRequest(); // We will have lost our selection from the paste operation. for (int i = 0; i < new_text.length(); i++) { page()->triggerAction(QWebPage::SelectPreviousChar); } } The debug print shows exactly what Sigil was provided by the javascript: Debug: In BookViewEditor "First Chapter Second Chapter" So there isn't even an easy way for me to tell that a user has highlighted more than just a piece of text. I will have to think about how to get the selection in some other way that will let me see if it passes over tag boundaries or not. This one won't be an easy fix or even fixed in the next release. KevinH

05-11-2016, 10:59 AM	#12
Turtle91 A Hairy Wizard Posts: 3,313 Karma: 20171571 Join Date: Dec 2012 Location: Charleston, SC today Device: iPhone 15/11/X/6/iPad 1,2,Air & Air Pro/Surface Pro/Kindle PW & Fire	I changed my coding procedures to allow for title case in my TOC and all caps in the chapter. I simply code the title in the chapter how I want it to be displayed in the TOC. Then change the CSS to make the chapter all caps. The auto TOC doesn't take into account the CSS so everything looks fine. eg: Code: h2 {text-transform:uppercase} <h2>Title Case of a Particular Chapter</h2> How it appears: TOC: Title Case of a Particular Chapter Chapter Head: TITLE CASE OF A PARTICULAR CHAPTER

05-10-2016, 02:38 PM	#5
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	Huh ... Where are the font color tags coming from? They must be internally used inside of BookView as they are not in your source. Wow! That is a strange one. I'll see if I can track down what BookView is doing and get it fixed. Thanks for your bug report. KevinH

05-11-2016, 12:19 AM	#9
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	This bug has been around a very long time - since day one of BookView. The issue is people selecting things that cross-over text boundaries unknowingly in BookView. The buttons do work as advertised if the user properly selects just the text they want to change the case of and does not stretch across tags. So I hesitate to disable it for BookView but might if I can't come up with something that works. There are just so many many things that can go wrong when editing in BookView, that I think its days are numbered. KevinH

05-11-2016, 12:41 AM	#10
AlanHK Guru Posts: 681 Karma: 929286 Join Date: Apr 2014 Device: PW-3, iPad, Android phone	It's completely unacceptable for links to be deleted. Something that destroys code without warning without any "undo" is more a liability than a useful feature. Better to disable anything that does that in bookview. What I ideally want is a way to edit the text without being confused by a swarm of code; that leaves the code between the displayed text unchanged. I realise that this becomes problematic if you start cutting and pasting text around from one place to another, joining paragraphs which necessarily restructures the code. But the special case of changing case, there is no reason to change any formatting code at all. I thought if it was allowed it must be safe.

05-11-2016, 10:19 AM	#11
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	As I said, if it is truly "unacceptable" to highlight too much and then paste over the top of it just text and then be surprised that it overwrites non-text then do NOT work in BookView. This can happen in many places and ways in BookView as there is no way to easily undo it since you are effectively changing a DOM tree inside a browser window. And again as I said in my last post I will look into the best way to handle this case. KevinH

05-11-2016, 12:14 PM	#13
KevinH Sigil Developer Posts: 8,487 Karma: 5703586 Join Date: Nov 2009 Device: many	Fixed in master by checking if a user selects more than just text in BookView and if so silently refusing to change case (and thereby do damage). KevinH

05-11-2016, 12:35 PM	#14
DiapDealer Grand Sorcerer Posts: 28,362 Karma: 203720150 Join Date: Jan 2010 Device: Nexus 7, Kindle Fire HD	Thanks, Kevin. Glad you didn't have to disable it completely.

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
Change CAse for ToC?	BaconButty	Editor	13	10-04-2015 01:21 PM
KB Shortcuts for Change Case?	phossler	Editor	12	08-15-2014 05:20 AM
Change case tool	ZoeRochelle	Editor	12	04-04-2014 06:55 PM
My "quest" for a JBL case (Case\Cover/Sleeve\Skin links)	tomereader	Ectaco jetBook	18	12-02-2011 02:31 PM
Folder case change	skells	Calibre	3	05-17-2011 02:24 PM