1117726 - Garmin Connect website hides dropdowns on progress summary page

Status: RESOLVED WONTFIX

(Web Compatibility :: Site Reports, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Whenever I'm on the mini report page (http://connect.garmin.com/modern/reports) and want to see a specific subset of my activities, both the dropdowns for time and type get hidden.

Steps:
1. Login to Garmin Connect
2. Go to reports -> progress summary
3. Choose an entry from one of the two dropdowns (time or activity type)

After step 3 both the above mentioned dropdowns are not visible anymore, and you are not able to do any other selections.

I checked this with a recent Nightly but also with version 31 of Firefox. Both are affected. Not sure yet if this is a regression on our side or in Garmin connect. Other browsers like Safari don't show the same behavior.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is actually a regression between Firefox 10 and 11. I will check for a regression range.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Last good revision: 6d42793c4807 (2011-11-14)
First bad revision: 9ae1d4f44b8b (2011-11-15)

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6d42793c4807&tochange=9ae1d4f44b8b

I will try to nail it further down to the specific changeset.

Comment 3

[Hallvord R. M. Steen [:hallvors]]

No clues in the console?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

No failure is shown in there. Not sure when exactly I will find the time to continue bisecting given that I have to build Firefox myself for that. If someone else has time, I would appreciate it.

Comment 5

[Hallvord R. M. Steen [:hallvors]]

There is some error about a non-node being passed to document.evaluate() - might be a red herring, but I can reproduce the problem and will keep looking.

Comment 6

[Hallvord R. M. Steen [:hallvors]]

I'm somewhat mystified here. Stepping through the script, these controls seem to disappear the moment send() is called on an XHR object - without stepping into any JS that looks like it would be responsible for making them disappear.. Some in-progress observations though:

The change event handler will do this:

$(ReportEventHandlerConstants.REPORT_SUBMIT).click()

which triggers onclick for this display:none button (sort of weird that you can even do that):

<input type="button" style="display:none;" onclick="A4J.AJAX.Submit('reportFilterForm',event,{'similarityGroupingId':'reportFilterForm:hiddenReportSubmit','parameters':{'reportFilterForm:hiddenReportSubmit':'reportFilterForm:hiddenReportSubmit'} ,'actionUrl':'/minreports'} );return false;" name="reportFilterForm:hiddenReportSubmit" id="reportFilterForm:hiddenReportSubmit">

As it is an <input type=button> the return false at the end is not really required but it shouldn't harm either.

Comment 7

[Hallvord R. M. Steen [:hallvors]]

The underlying cause here seems to be in Gecko's node.outerHTML setter. Unsurprisingly, this setter is going to assume that the code being set is *HTML*, not XHTML or XML. Now, when we run into this charming logic in the oldish Sarissa library:

      if (!window.opera && !A4J.AJAX.isWebkitBreakingAmps() && oldnode.outerHTML && !oldnode.tagName.match(/(tbody|thead|tfoot|tr|th|td)/i)) {
        LOG.debug('Replace content of node by outerHTML()');
        if (!Sarissa._SARISSA_IS_IE || oldnode.tagName.toLowerCase() != 'table') {
          try {
            oldnode.innerHTML = '';
          } catch (e) {
            LOG.error('Error to clear node content by innerHTML ' + e.message);
            Sarissa.clearChildNodes(oldnode);
          }
        }
        oldnode.outerHTML = new XMLSerializer().serializeToString(newnode);

the generated XHTML code will contain stuff like 
        <div class="menuDivider"/>
inside a <div style="display:none"> - the self-closing syntax most likely won't be recognised and much more than expected of the DOM tree will end up being display:none.

I think there may be some more bugs with outerHTML setting, because it seems the SELECT elements are not actually in the re-generated DOM at all.. but I think it makes sense to recommend that Garmin updates this code to make it more sensible and robust..

Comment 8

[Hallvord R. M. Steen [:hallvors]]

To be clear: the simple fix is to adjust the code so that Firefox runs the one described as "Replace content of node by replaceChild()" instead of the outerHTML one. Given all the odd exceptions in the if clause in comment 7 and the state of modern browsers, I think it would be better to use replaceChild() by default for all browsers..

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hallvord, thank you a lot for this investigation! I appreciate and hope that we can get Garmin to fix that nasty issue.

Comment 10

[Hallvord R. M. Steen [:hallvors]]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Last good revision: 6d42793c4807 (2011-11-14)
> First bad revision: 9ae1d4f44b8b (2011-11-15)
> 
> Pushlog:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=6d42793c4807&tochange=9ae1d4f44b8b

(Obviously the "culprit" was:
Henri Sivonen — Bug 92264 - Implement outerHTML. Part of the patch by Ms2ger. r=smaug.)

Comment 11

[Adam Stevenson [:adamopenweb]]

Looks like this same (or very similar) issue was reported on webcompat.com. The result from outreach was to close as WONTFIX. 

"There is no team working on the reports feature. The feature is planned to be removed from the site, they did not give a time frame."

https://github.com/webcompat/web-bugs/issues/2998

Closing this issue.