Auto Closing Divs?


(Logan Mathews) #1

To theme my installation of discourse, I’ve wrapped the site in a few divs (and included others). In the customization -> top tab I have a few open ended divs which are closed manually in the /body tab. This worked well in beta 2 and 3 but recently broke in a newer update. Now, for some reason, open ended divs in the ‘top’ tab are automatically being closed.

If I move the content of the ‘top’ tab to the header, the divs are not prematurely closed and things work almost as they should… except the discourse header then is also incapsulated in my formatting, which I didn’t want.


(Kane York) #2

If you need to wrap the Discourse content in extra divs for styling, you should contribuite that to core. Look in app/assets/javascripts/discourse/templates.


(Logan Mathews) #3

Oh I totally agree that some changes are better suited for editing of the core. My question is this.
The header tab doesn’t autoclose divs.
The top tab didn’t close divs in the past.
The top tab closes divs as of a somewhat recent patch.

Why is there in-congruent behavior between the header and top sections? Why was the top changed? If it was changed to avoid people breaking layout due to forgetting to close divs, that sounds like an end-user problem. Removing functionality to prevent people from ‘hurting’ themselves when there are demonstrable benefits to it’s past implementation seems foolish.


(Kane York) #4

I think it’s up to the browser whether to autoclose or not… the HTML just gets inserted as a string by the JS.

If you don’t trigger quirks mode behavior, you won’t get quirks mode results.


(Logan Mathews) #5

It would seem odd for it to be a browser issue as the header section does not auto-close the divs in the same browser.


(Logan Mathews) #6

I have verified that the header section is handled differently (and ideally) from the top and footer sections, regardless of my web browser.
The header customization input field does not auto fix divs while the other 2 mentioned do.

The header custom html is loaded/printed directly into the source of the page. The other 2 are done through
PreloadStore.store(“customHTML”,{“top”:“Here”,“footer”:“Here”});


(Mittineague) #7

I’ve been looking at this myself today and you are correct about where the Admin Customize are inserted. eg. compare the mark-up in view-source with the generated mark-up

  <body>
    <noscript data-path="/t/visual-distinction-between-reply-to-last-comment-and-reply-to-original-post/27501/24">
.....
    </noscript>
    <!--[if IE 9]><script type="text/javascript">ie = "new";</script><![endif]-->
    <section id='main'>
    </section>
    <div id='offscreen-content'>
    </div>

i.e. the H3 "This is in …"s

So indeed, any malformed mark-up added in to Admin Customize Header could produce bad effect


(Logan Mathews) #8

I actually like the way customize header does it and dislike how the other sections sanitize. The admin should be responsible enough to not botch the layout/etc. At the very least making it a toggle’able ‘sansitization’ would be a start.


(Mittineague) #9

Yes, they may not produce bad effect, but until one knows what the sanitization is doing they could produce unexpected effect.


(Logan Mathews) #10

I’m glad someone else has noticed this inconsistency ;).

If you happen to find out where in the code this is happening (on their github page), please let me know :D. I’ve gone through the application files and it seems like everything should be handled in the same fashion, but they’re obviously not. If you’re able to find out where the difference is between the header/top/footer/body is happening, I’ll be glad to fix it (since I think we want different out-comes)


(Mittineague) #11

With over 7K files I’ve yet to see them all let alone understand them. But my best guess is this is the one to start from

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/lib/html.js


(Logan Mathews) #12

I actually saw that file too and thought it’d be a start. What I’m unsure of is where the head and the other sections are being treated differently.


(Mittineague) #13

This looks even closer


(Logan Mathews) #14

Definitely getting closer now :). Just trying to find out specifically when the change to custom-html started auto-closing divs. Most of the files seem to match my old copies of them, but I’ll figure it out soon. :smiley:

As far as if people wanted to sanitize the header too (ugh), i suppose {{custom-html “header”}} or something similar would get the job done.

I guess I know what I’ll be searching through tomorrow


(Mittineague) #15

I’m about to head off to sleep, but the dialects folder looks promising as well
https://github.com/discourse/discourse/tree/master/app/assets/javascripts/discourse/dialects


(Logan Mathews) #16

Have a great night and thanks again for pointing me in the right direction.

It seems like its an easy fix to make everything match. I guess it’s a question of IF each section should automatically fix non-closed elements or not. Hopefully not but I guess we’ll have to wait for someone like @sam to state which section (header or top/footer/bottom) is acting correctly.

I personally hate having to fight a system that thinks my input is flawed. Since this is an admin only side thing, I’d hope that there’d be less assumptions and more flexibility.


(Mittineague) #17

I’m mixed, on one hand I believe if someone doesn’t know what they’re doing they shouldn’t do it unless they’re ready to face the consequences (thank heavens I have a localhost to muck about with). On the other hand, I’ve seen quite a few that were seeking help after they crashed their live WordPress site by editing files in the ACP

Seems to me that the difference between development and production is obvious, but give some a tool and right or wrong they’ll use it.


(Logan Mathews) #18

Oh I agree. I think a toggle in the interface somewhere (safe by default I suppose), would be a fair compromise.


(Mittineague) #19

Definitely something has changed. My earlier plugins using plugin-outlet have broken either in part or completely.

Looking into one, I see comments I didn’t notice being there before and unwanted div tags be inserted.

<ul class="location-links">
  <li></li>
  <li></li>
  <li></li>
  <li></li>
  <li></li>
  <!---->
  <div id="ember1959" class="ember-view site-map-links-outlet site-map-links">
    <li>
      <a href="http://localhost:4000">Home</a>
    </li>
  </div>
  <li></li>
  <li></li>
  <li></li>
  <!---->
</ul>

It “works” but I don’t like seeing div tags inside a ul


Plugin-outlet incorrectly handles multiple plugins at the same outlet
(Logan Mathews) #20

It seems that whatever was changed may be having more implications than people could of forseen. I am somewhat shocked to not see an official comment yet on if this is working as intended or not.

@codinghorror Any idea why/when this changed? Is it working as intended? Why is there inconsistencies between the html injection points?