360746 - [FIX]The right panel has disappeared at andrewdupont.net

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Martijn Wargers (dead)]

This is a follow-up from bug 358804, comment 1.
There should be a right panel at the right top of the page, but it isn't anymore with current trunk builds.
This regressed between 2005-12-08 and 2005-12-09:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-12-08+05&maxdate=2005-12-09+06&cvsroot=%2Fcvsroot
I think this is a regression from bug 318128, I'll attach a testcase that shows the difference in behavior.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmmm.  So first of all, I get a somewhat different regression range:  2005-12-09-06 to 2005-12-10-06:  http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-12-09+06&maxdate=2005-12-10+06&cvsroot=%2Fcvsroot

In that range, there is a checkin which indeed leads to this difference: bug 303313.

What happens now is that we go through and set the <link> to be enabled, which sets the sheet enabled.  Then we start a new sheet load due to the rel change, which causes a _new_ sheet with that title to start loading... and that sheet ends up disabled, because it has a different title from the preferred sheet set.

I suppose we could try not doing a reload for this particular rel change.  Or we could try somehow having the disabled state of the <link> override what's going on with the sheet...  Not quite sure which behavior we want, e.g. if the url changes in this case instead of the rel.  What do other UAs do?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ian, you haven't happened to have specced this, by any chance?

Comment 4

[Hixie (not reading bugmail)]

http://www.whatwg.org/specs/web-apps/current-work/#styling
http://dev.w3.org/cvsweb/~checkout~/csswg/cssom/Overview.html#dynamically

Comment 5

[Boris Zbarsky [:bzbarsky]]

I'm not sure that covers what I'm asking.

What I'm asking are two questions:

1)  Should a change of "rel" from "alternate stylesheet" to "stylesheet" have any effect?  If so, what?

2)  If an alternate (so disabled) sheet gets enabled via the DOM api and then the <link> is changed such that a new sheet load has to start (change of href, possibly change of rel, possibly change of title, etc), should the new sheet that loads have the same state as the old sheet, or should it be different somehow?

Comment 6

[Hixie (not reading bugmail)]

1) I don't think so, assuming the stylesheet has a title. Whether the stylesheet is a preferred stylesheet or an alternative stylesheet only matters when the stylesheet is inserted into the document; once its in, only its title matters.

If it doesn't have a title, it doesn't make any difference, because it's enabled either way (regardless of the presence of an 'alternate' in the rel="").

2) Changing href="" doesn't cause anything to load. Changing rel or title shouldn't change if it loads or not. It's not immediately clear to me what should happen to the disabled state if rel or title are changed, the CSSOM spec is extremely vague about those attributes. I'll forward your questions to the guy editing that spec.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I'm assuming you'll add those answers to the spec explicitly in some way, right?

That sais....

> 2) Changing href="" doesn't cause anything to load.

That seems very odd to me.  Quite apart from introducing hysteresis into the system, that's not what Gecko, Opera, or Konqueror do.  And I seem to recall sites actually setting href dynamically.

Comment 8

[Hixie (not reading bugmail)]

Uuh. I meant that changing href="" wouldn't cause the disabled attribute to change. Sorry. I must have been thinking of the other attributes when I wrote that and got myself confused.

The spec in question isn't undre my editorship, but I've asked Anne to update the spec apprioriately, yes.

Comment 9

[Boris Zbarsky [:bzbarsky]]

But changing the |disabled| property on the linking node doesn't actually set an attribute on the node.  Is the point that the property should be stored on the node and not on the sheet?  If so, how does that interact with the sheet's |disabled| property?

Comment 10

[Hixie (not reading bugmail)]

Oh, I see. Good point. What does IE do? I'm guessing Anne will have to spec that. But yeah, that's a question for Anne.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 262599 [details] [diff] [review]
Proposed fix

Wait, don't you need to also enable the sheet if rel goes from not containing "stylesheet" to doing so?

Comment 13

[Boris Zbarsky [:bzbarsky]]

That will happen, yes.  If the second arg is false (which it will be in this case), then we'll do the compare of the current URI to the sheet URI... except there is no sheet, so no sheet URI, so we won't bail out early and will proceed with the sheet load.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 262599 [details] [diff] [review]
Proposed fix

awesome

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 16

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008020819 Minefield/3.0b4pre and the testcase from this bug

- Verified fixed