Tizra / Tizra-Customer-Tracker

Bug/Enhancement tracking for Tizra publisher project
3 stars 0 forks source link

New styles you added break our CSS on some site's title content pages #127

Closed orangedrink closed 11 years ago

orangedrink commented 11 years ago

To see what I mean see this page on TRB http://amonline.trb.org/1s7ke4/1

If possible I'd like to see this remedied without having to check all of our sites and fix each one.

Thanks.

daviddurand commented 11 years ago

You didn't say what the problem was, but when I switched to PDF view rather than page view, I did see two images of the page. The extra image seems to be markup placed inline by some Javascript, I'm guessing, as it is not in the markup Tizra generates, as shown by a view source.

I need you to explain what is going on here. I may well be able to do something that will help (like change an ID or an HTML class), if you can tell me what your script is trying to do, and how it's going wrong.

In any case, I would recommend revising your designs, because whatever is supposed to be going on, embedding page images in that way is going to cause problems.

The smart block is responsible for displaying the page content, and it does a lot of important size calculations, and browser platform adjustments to ensure that the page is displayed in the most efficient way,and that all features will work properly. I took a lot of care to try to make the reader changes low-impact (for instance introducing new divs inside of elements that were previously used in custom style sheets), etc. The new structure is required for the correct operation of the new link overlays that make PDF links work for image displays. Sometime soon, we may have drag and copy text selection in image mode, and shortly thereafter we will start explicitly retiring the use of the PDF plugins altogether.

I assume that in this case the image being fetched shouldn't be displayed (as image mode is off). So even if the stylesheet hides it, it will just raise server load, and make every pdf page slower. If are just trying to lock the site into image mode only, that is something that can be done with an advanced site property. I've enabled it for the site in case that is useful.

daviddurand commented 11 years ago

I need answers to the questions I asked above if I am to have any hope of helping you.

If you can explain what you are trying to do, I will see if there is a change that I can make to make your life easier. The new element is necessary for links to function. I'm not looking for instructions on what to change, but a clear description of a. exactly what you are doing, b. why it's broken, and c. why you wanted to do it. I think a and b are obvious -- I can't spend hours trying to guess what's wrong just by reading your code. C is particularly important since it will at least let us find the easiest fix, even if it's at your end.

Using CSS to modify formatting and javascript to decorate elements is something we expect. However, if your styles depend intimately on every detail of the markup that a block creates (rather than the basic element classes that we've created to make customization possible), there may sometimes be issues. I have tried to make sure that the classes that people were invited to override via CSS have been attached to elements that can still be, for instance, resized.

Since the duplicated content is not even in the markup that the system delivers, there is no question that whatever you are doing is not something that Tizra can offer compatibility guarantees for. And in any case, sometimes important updates (like the new link support) may require updates if extensive customizations have been made. We don't change the markup very often, but we don't make guarantees that nothing will ever change.

orangedrink commented 11 years ago

A: We are trying to position the PDF over the image.

B: I don't know. It was working when we designed it and now it isn't. We don't get paid by our customers to fix things that are broken by your updates so I'm not really able to spend any time figuring that out.

C: We are trying to position the PDF over the image so that if there is a pop up dialog we can hide the PDF and impact the look of the page as little as possible. I've also used this in place of the image/pdf selector on these older sites as well. This has the added benefit of the PDF showing through on some browsers when the user is missing a pdf plugin.

orangedrink commented 11 years ago

Here's another site suffering from the same problem. http://aaoscoursematerials.com/1tqtmr/1tqtmr/1

orangedrink commented 11 years ago

MPS as well http://mps.omnipress.com/18sd8q/18sd8q/1

FYI we're cancelling upcoming customer demos now because we aren't confident that this will work.

daviddurand commented 11 years ago

Well, I'm sorry, but we've built a system for switching between images and pdf. If you had stuck with that, there would not a problem. As it happens, we are soon to discourage if not discontinue support for the PDF plugins as they are not reliable in terms of scrolling, size, and even existence.

We don't generate any markup to pull in an image, so none of what you are talking about makes sense in terms of product support. I can't help you to figure out how to fix the code you built to work around the code we actually support, unless you are going to pay me to. If you can explain what you are doing, I can help you to get back to something supported (and supportable). I may even be able to do something at this end to help, as we want your sites to work properly too. But if you are going to build something weird on top of our system, I can't make any guarantees that it will continue to work.

This problem is perhaps also going to run into Ryan's issue with CSS, which is something that we have suggested and offer some support for -- so there may be some markup or reader configuration changes coming.

daviddurand commented 11 years ago

Delete these lines from http://s3.amazonaws.com/kc-assets/TRB50220/trb-kc-5-22.js

if(j('#pdf embed ,#pdf object').length) {
    var filename = window.location+"";
    //alert(filename.replace(/^(.+)\?.+$/,"$1")+".jpg");
    j('#pdf .bounds').prepend("<img id='pdf_image' src='"+filename.replace(/^(.+)\?.+$/,"$1")+".jpg' alt='' />").find('#pdf_image').css('width',j('#not-sidebar embed.t-pdfpage,#not-sidebar  object.t-pdfpage').attr('width')).css('height',j('#not-sidebar embed.t-pdfpage,#not-sidebar  object.t-pdfpage').attr('height'));
}

Obviously I can't do this. It does not seem to be shared across sites (judging from the name) so perhaps you could store it on the server where it could use browser caching. In any case, I think that the graphic is changing the layout of the PDF image.

In the meantime, I have fixed the site by clicking the button that I added for you two days ago. Now it's in image-only mode, and it's working.

orangedrink commented 11 years ago

Can you enable that setting across the board or at least on the other two sites linked to above?

adane commented 11 years ago

I've added the property to...

http://aaoscoursematerials.com

and

http://mps.omnipress.com

...and have set the property to force use of images on both.

I see that the property is already in place on TRB.

orangedrink commented 11 years ago

Thanks Abe.

This was a tough one to figure out but it looks like some styles and scripts we use on this page were broken when the markup for the embed tag was changed to:

<embed id="t-page-image t-pdfimage" width="775" height="1003" src="/workplace_conditions_culinary/1.pdf" type="application/pdf">

It looks like you are trying to add multiple IDs? Maybe you just want a space in the ID but either way that's bad markup.

This is markup from this page: http://ccfcs.tizrapublisher.com/workplace_conditions_culinary/1 Just so you know this isn't something that we are doing with a script or whatever.

Let us know when you have this markup how you want it so we can start to rework all these old sites.

orangedrink commented 11 years ago

Until this is fixed our sites are broken and our hands tied. Please hurry.

daviddurand commented 11 years ago

The markup error (two IDs) was fixed about at 1:30 pm today, when I noticed it in the ASPPA pages, and it should have been live within at most an hour after. It was introduced last night at 1:30 AM, as I was trying to make some changes to help with #128 so that at least the solution in #100 would still be possible (CSS).

I can't guarantee that the won't need to be some further tuning, but I think that the structure of PDF and image display are basically right at this point (and the new linking features that led to the new <div> being introduced are working pretty well, though there may still be some PDFs that confuse things).

You might be interested to know that one reason I changed the classes on the PDF blocks is that some did not have t- prefixes (should should have been off-limits) -- and worse, the classes and the nesting were both sometimes different for some of the combinations of user-options, browsers, and display contexts (zoomViewer, new reader, page block). So the status quo may have had bugs in any case. The new setup is much more uniform.

orangedrink commented 11 years ago

Well then I'm still not sure what the real problem is.

Anyway maybe you could make time to look over our sites and get familiar with what we're doing so that in the future you can avoid breaking our designs or at least give us a heads up if something you're doing will impact us. I know we've asked to be notified whenever you make any updates but I guess that's a big request.

If you have any clear guidelines on what it's OK for us to depend on in the future and what you're planning to change that would be helpful.

Also any other ideas you might have on how we could prevent problems like this down the road would help give us all some peace of mind on this end.

Thanks