<IMG> height attribute doesn't work...



  • <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="50"/>

    <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="100"/>

    <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="200"/>

    [spoiler]<img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="100"/>[/spoiler]
    [spoiler][/spoiler]



  • There is something weird going on with the height attribute, but IDK what it is. Those 3 images should all be different sizes, and they are in the Preview...

    Looks ilike the height of the last image 'wins' (except if it's [spoiler]ed...)

    :wtf:


  • sockdevs

    <img src="/uploads/default/16426/59de84ca58b55ec0.png" width="100">
    <img src="/uploads/default/16426/59de84ca58b55ec0.png" width="200">
    <img src="/uploads/default/16426/59de84ca58b55ec0.png" width="300">
    

    Width is the same…


  • sockdevs




  • sockdevs







  • edit: That still works, at least.


  • sockdevs

    <img src="/uploads/default/16021/ae5e4d6753acd9ff.jpg" width="100">
    <img src="/uploads/default/16021/ae5e4d6753acd9ff.jpg" height="200">
    

  • sockdevs

    …yep: it's fucked :laughing:


  • sockdevs

    Repro'd and reported at meta.d:

    The onebox needs rebaking…



  • <img src=http://what.thedailywtf.com/uploads/default/_emoji/fa_thumbs_up.png?v=0>


  • sockdevs

    Hmm… what if the images are different?


    Well… there's a thing…

    Updated meta.d to include this new info.



  •   def limit_size!(img)
        # retrieve the size from
        #  1) the width/height attributes
        #  2) the dimension from the preview (image_sizes)
        #  3) the dimension of the original image (HTTP request)
        w, h = get_size_from_attributes(img) ||
               get_size_from_image_sizes(img["src"], @opts[:image_sizes]) ||
               get_size(img["src"])
        # limit the size of the thumbnail
        img["width"], img["height"] = ImageSizer.resize(w, h)
      end
    
      def get_size_from_attributes(img)
        w, h = img["width"].to_i, img["height"].to_i
        return [w, h] if w > 0 && h > 0
      end
    

    The first option in the || chain explains what you see when you give both width and height.

    The second option explains how it's using the size of the last image in the post.



  • I'm not saying it was Discourse, but it was Discourse!

    My ruby is rusty, so after I've had a doctor take a look at it, I'll probably defer to your analysis.

    Having a function called get_size_from_image_sizes() seems a bit odd to me. Having said that just looking at the general form of this code

    w, h = (blah1)  || (blah2) || (blah3);
    

    is giving me a headache. I don't think I even want to know what happens if one of those get_size_*() functions returns a single value rather than a pair... :/


  • sockdevs

    IIUC, because the three tags point to the same image, whatever it is that has the image sizes only has one entry?


  • :belt_onion:

    <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="50" class=emoji />

    take that.

    <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/220/12008.png" height="100%"/>

    <img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/200/12008.png" height="200"/>

    [spoiler]<img src="http://what.thedailywtf.com/user_avatar/what.thedailywtf.com/tar/120/12008.png" height="120"/>[/spoiler]
    [spoiler][/spoiler]


  • :belt_onion:

    no repro, diff sizes working ;)


  • sockdevs

    You might want to read the thread properly:stuck_out_tongue:


  • :belt_onion:

    <!-- do i seriously need to :trollface: my trolling here -->
    I care not to explain these things.



  • Basically: If you don't specify both width and height, Discourse downloads the image and throws out your numbers.


  • :belt_onion:

    <!-- whoosh -->



  • :^)

    <!-- lol body is invalid -->

  • :belt_onion:

    damnit, if you have the [1 Reply] expanded, the [</>] view raw button doesn't work


  • Winner of the 2016 Presidential Election

    @darkmatter said:

    damnit, if you have the [1 Reply] expanded, the [</>] view raw button doesn't work

    WUT?

    What did they do to the poor DOM now? Sigh... I'll take a look...



  • @Onyx said:

    What did they do to the poor DOM now?

    It looks like normal has:
    [code]

    
        
    [/code]

    But with an expanded quote you get:
    [code]

    
        
    <section class="embedded-posts bottom"> </section>
    [/code]

  • Winner of the 2016 Presidential Election

    Ah, additional layer. Probably used .children() instead of .find(). That's easy to fix at least.


  • area_deu

    When'd that break?


  • Winner of the 2016 Presidential Election

    Oh hell, forgot about this... @PJH, when you have the time, mind trying changing the raw button code to this?

    <script>
    // Show Raw button
    //http://what.thedailywtf.com/t/expanding-replies-hides-raw-button/7399/47?u=pjh
    (function(){
      //@Monarch at what.thedailywtf.com
      //Button is redelcared and sligthly modified due to new scope. it can be further simplified.
      Button = function(action, label, icon, opts) {
        this.action = action;
        this.label = label;
    
        if (typeof icon === "object") { this.opts = icon; } else { this.icon = icon;}
        this.opts = this.opts || opts || {};
    
        this.render = function (buffer){
            var opts = this.opts;
            buffer.push("<button title=\"" + this.label + "\"");
            if (opts.className) { buffer.push(" class=\"" + opts.className + "\""); }
            buffer.push(" data-action=\"" + this.action + "\">");
            if (this.icon) { buffer.push("<i class=\"fa fa-" + this.icon + "\"></i>"); }
            buffer.push("</button>");
        }
      };
    
    
    
      Discourse.PostMenuView.reopen({
          buttonForRaw:function(post){
              return new Button('raw', 'view raw post', 'code', {className: "raw-button", disabled: false});
          },
    
          clickRaw:function(post){
              var topicID = post.topic_id,
                  postID =  post.post_number,
                  postArea = $("article[data-post-id='"+post.id+"'] div.contents"),
                  $rawButton = $(this.element).find("button.raw-button"),
                  styles = [{backgroundColor: 'transparent', color: '#A7A7A7'},{backgroundColor: '#08C', color: '#FFF'}];
    
              if (postArea.find('.tdwtf-raw-area').length == 0){
                  var postArea_raw_content = $('<pre class="tdwtf-raw-area"></pre>'),
                      cooked = postArea.find('.cooked');
    
                      cooked.after(postArea_raw_content);
    
                  $.get('/raw/' + topicID + '/' + postID) .done(function (content) {
                      postArea_raw_content.addClass("active");
                      $rawButton.css(styles[1]);//active
                      postArea_raw_content.css({"white-space":"pre-wrap", 'border':'2px dashed #E7E7E7','padding':'3px'}) .text(content);
                      cooked.hide();
                  });
              } else {
                  var postArea_raw_content = postArea.find('.tdwtf-raw-area');
                  if ( !postArea_raw_content.hasClass("active") ){  //raw no active
                      postArea.find('.cooked').hide();  
                      postArea.find('.tdwtf-raw-area').show();
                      postArea_raw_content.addClass("active");
                      $rawButton.css(styles[1]);
                  } else{
                      postArea.find('.cooked').show();  
                      postArea.find('.tdwtf-raw-area').hide();
                      postArea_raw_content.removeClass("active");
                      $rawButton.css(styles[0]);
                  }
              }
          }
      });    
    })();
    </script>
    

    I think that should do it.

    I'll add this to a GitHub repo later, I have to run now...


  • Discourse touched me in a no-no place

    I'll need reminding...


  • sockdevs

    when would you like the reminder?



  • @todo notification function?



  • Todo a8e4bb6e, (PleegWat:uncategorized) notification function?

    <!-- Posted by SockBot 0.16 "Hazardous Hera" on Sat, 04 Apr 2015 15:10:30 GMT-->

  • Discourse touched me in a no-no place

    @PJH said:

    I'll need reminding...

    Done.


  • Winner of the 2016 Presidential Election

    @PJH said:

    Done.

    And survey says... it works!

    Looks good on my end.


  • area_deu

    Doesn't seem to work for me for some reason.

    Also, the style selector seems a little wrong(@PJH):


  • Discourse touched me in a no-no place

    @aliceif said:

    Also, the style selector seems a little wrong(@PJH):

    How? I can't immediately see anything wrong...


  • area_deu

    It says that "Widescreen+min" is selected. Even though it obviously isn't (look at the post action buttons).


  • Winner of the 2016 Presidential Election

    Hard refresh? There is nothing browser-specific in the code, shouldn't be affected.

    Since I didn't actually put it on GitHub (bad @Onyx!) I don't have a diff, but the change was basically replacing all .child() selectors with .find(), which means that the elements will still be matched even after Discourse wraps posts in another element (which happens when you expand the quotereply-to).

    @PJH said:

    How? I can't immediately see anything wrong...

    The asterisk is pointing at the wrong style. I can confirm this.


  • area_deu

    According to the Firefox Devtools, the version with the .find()s seems to be loaded.


  • Discourse touched me in a no-no place

    @aliceif said:

    It says that "Widescreen+min" is selected. Even though it obviously isn't (look at the post action buttons).

    Oops....


  • Winner of the 2016 Presidential Election

    Well, just tested in Iceweasel, works fine.

    Do you have any userscripts loaded that might interact with it? I don't see a reason why it wouldn't work, and can't repro it in Opera nor Iceweasel. I tried it on topic load, on Infiniscroll™ loaded posts and after a hard refresh, all cases behave as intended.

    OT: Iceweasel's font rendering is horrendous!


  • Discourse touched me in a no-no place

    @PJH said:

    Oops....

    Fixed. Will need a hard refresh to reload the JS.


  • area_deu

    @Onyx said:

    Well, just tested in Iceweasel, works fine.

    I opened Chromium (which has no add-ons installed), deleted all data, opened this thread and still couldn't show raw for posts with opened replies (downward).
    It works with opened replies (upward), though.
    This just confuses me.


  • Winner of the 2016 Presidential Election

    @aliceif said:

    with opened replies (downward)

    D'OH!

    Sorry, confirmed, I was just testing the upwards one. I almost never use the downwards one and I completely forgot to test that!

    I'll take a look at it. Damn Discoursistency, can't they have a single way of wrapping elements... *grumble*


  • area_deu

    @darkmatter said:

    damnit, if you have the [1 Reply] expanded, the [</>] view raw button doesn't work

    @Onyx said:

    Sorry, confirmed, I was just testing the upwards one. I almost never use the downwards one and I completely forgot to test that!

    I think you missed the point of the bug report

    (And I replied to the wrong post ...)


  • Winner of the 2016 Presidential Election

    Indeed. But that doesn't change the fact that the other case was broken as well :laughing:


  • Discourse touched me in a no-no place

    @tar said:

    I don't think I even want to know what happens if one of those get_size_*() functions returns a single value rather than a pair...

    Promotions!

    The h will be set to nil because that makes sense. Sort of. I don't really care for Ruby…


  • mod

    Despite lack of comments on your meta.d bug, it looks like this may actually be fixed now:

    <img src="/uploads/default/18297/45c0ebe0accf40ab.png" height="200">
    <img src="/uploads/default/18297/45c0ebe0accf40ab.png" height="150">
    <img src="/uploads/default/18297/45c0ebe0accf40ab.png" height="100">
    

    Edit: Wait, I got it to work earlier, what was the incantation ...


  • sockdevs

    I saw it flash to the correct result, then it went back to wrong


  • mod




  • sockdevs




Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.