Last Comment Bug 550469 - [PageInfo] Sync changes from mozilla-central
: [PageInfo] Sync changes from mozilla-central
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Page Info (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-05 06:32 PST by Philip Chee
Modified: 2010-03-12 08:09 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 proposed patch. (10.11 KB, patch)
2010-03-05 06:42 PST, Philip Chee
neil: review+
neil: superreview+
Details | Diff | Review

Description Philip Chee 2010-03-05 06:32:27 PST
Port relevant patches from minefield pageInfo.

[Bug 494808] Page Info > Media doesn't list background images anymore.
[Bug 519390] Page info sometimes resizes an image incorrectly when it uses scaling.
[Bug 525190] Refactor the way pageInfo.js handles arguments.
[Bug 523047] Slow selectors in browser css)

I didn't bother with patches that dealt with the fallout from removing metadata.xul (because the code there was confusing and unmaintainable). And then repurposing PageInfo as a faux "View Image Properties". This mashup resulting in confusing and unnecessarily convoluted code (and they are /still/ at it).
Comment 1 Philip Chee 2010-03-05 06:42:44 PST
Created attachment 430614 [details] [diff] [review]
Patch v1.0 proposed patch.

Time to ask for initial review.
Comment 2 neil@parkwaycc.co.uk 2010-03-05 08:37:43 PST
Comment on attachment 430614 [details] [diff] [review]
Patch v1.0 proposed patch.

>-/*
>-richlistitem[feed]:not([selected="true"]) .feed-subscribe {
>-  display: none;
I was thinking, "why this change", and then, "why was it commented out?"...
(It appeared (without comment!) between 379183-6b.diff and 379183-7.diff)

>+  // check for background images, any node may have multiple
>+  var computedStyle = elem.ownerDocument.defaultView.getComputedStyle(elem, "");
>+  if (computedStyle) {
>+    Array.forEach(computedStyle.getPropertyCSSValue("background-image"), function (url) {
>+      if (url.primitiveType == CSSPrimitiveValue.CSS_URI)
>+        addImage(url.getStringValue(), gStrings.mediaBGImg, gStrings.notSet, elem, true);
>+    });
>+  }
This seems familiar, it seems my tree already had this change ;-)
Comment 3 Philip Chee 2010-03-05 09:28:01 PST
Comment on attachment 430614 [details] [diff] [review]
Patch v1.0 proposed patch.

UI touched so asking for sr.

> I was thinking, "why this change", and then, "why was it commented out?"...
> (It appeared (without comment!) between 379183-6b.diff and 379183-7.diff)
Yeah I tracked this down as well. Since there was no comment and Firefox doesn't have this and nobody has complained I thought it was time to rm.
Comment 4 neil@parkwaycc.co.uk 2010-03-05 09:46:31 PST
(In reply to comment #3)
> Firefox doesn't have this
Actually, it does have it (uncommented)
Comment 5 Philip Chee 2010-03-05 10:55:06 PST
So enable it or just leave it uncommented?
Comment 6 neil@parkwaycc.co.uk 2010-03-05 12:34:21 PST
Comment on attachment 430614 [details] [diff] [review]
Patch v1.0 proposed patch.

Sorry, click failure on my part.
Comment 7 Robert Kaiser (not working on stability any more) 2010-03-12 08:09:21 PST
Pushed as http://hg.mozilla.org/comm-central/rev/01b7cede7010

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