Last Comment Bug 360746 - [FIX]The right panel has disappeared at
: [FIX]The right panel has disappeared at
: regression, testcase
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86 Windows XP
: P2 normal (vote)
: mozilla1.9alpha4
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Hixie (not reading bugmail)
: Jet Villegas (:jet)
Depends on:
Blocks: 318128
  Show dependency treegraph
Reported: 2006-11-14 17:27 PST by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2008-02-09 14:57 PST (History)
13 users (show)
roc: blocking1.9+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (614 bytes, text/html)
2006-11-14 17:28 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Proposed fix (5.03 KB, patch)
2007-04-23 22:20 PDT, Boris Zbarsky [:bz] (still a bit busy)
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2006-11-14 17:27:08 PST
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:
I think this is a regression from bug 318128, I'll attach a testcase that shows the difference in behavior.
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-11-14 17:28:10 PST
Created attachment 245626 [details]
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2007-02-06 01:10:46 PST
Hmmm.  So first of all, I get a somewhat different regression range:  2005-12-09-06 to 2005-12-10-06:

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 [:bz] (still a bit busy) 2007-04-16 01:19:24 PDT
Ian, you haven't happened to have specced this, by any chance?
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2007-04-16 02:03:03 PDT
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) 2007-04-16 02:17:53 PDT
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 [:bz] (still a bit busy) 2007-04-16 10:37:24 PDT
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) 2007-04-16 13:40:53 PDT
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 [:bz] (still a bit busy) 2007-04-16 13:53:31 PDT
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) 2007-04-16 15:09:40 PDT
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 [:bz] (still a bit busy) 2007-04-23 22:20:07 PDT
Created attachment 262599 [details] [diff] [review]
Proposed fix
Comment 12 Jonas Sicking (:sicking) No longer reading bugmail consistently 2007-04-24 11:10:48 PDT
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 [:bz] (still a bit busy) 2007-04-24 11:42:55 PDT
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 2007-04-24 11:52:31 PDT
Comment on attachment 262599 [details] [diff] [review]
Proposed fix

Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2007-04-25 11:48:28 PDT
Checked in.
Comment 16 Carsten Book [:Tomcat] 2008-02-09 14:57:00 PST
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

Note You need to log in before you can comment on or make changes to this bug.