Not detecting feed when type has leading and/or trailing whitespace

RESOLVED FIXED in seamonkey2.17

Status

SeaMonkey
Feed Discovery and Preview
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Robert Kaiser, Assigned: ewong)

Tracking

Trunk
seamonkey2.17

SeaMonkey Tracking Flags

(seamonkey2.6 wontfix, seamonkey2.7- wontfix, seamonkey2.8 wontfix, seamonkey2.9 wontfix, seamonkey2.15 fixed, seamonkey2.16 fixed, seamonkey2.17 fixed)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
feed_discovery.html ported from Firefox in bug 484187 uncovered that the following test case doesn't work correctly in SeaMonkey right now and is therefore marked todo() for now:

    <!-- type can have leading and trailing whitespace -->
    <link rel="alternate" type="  application/atom+xml  " title="todo11" href="/11.atom" />
Hum, time to complete this, I think ;-<

I'm not familiar with this code, but I tried to have a look, fwiw:
http://mxr.mozilla.org/comm-central/ident?i=isValidFeed
*utilityOverlay.js: isValidFeed() regex (and code) are identical.
*feeds.js: initFeedTab() regex (and code) are different...
*...
status-seamonkey2.6: --- → wontfix
status-seamonkey2.7: --- → affected
status-seamonkey2.8: --- → affected
tracking-seamonkey2.7: --- → ?
Component: UI Design → Feed Discovery and Preview
QA Contact: ui-design → feed.handling

Comment 2

6 years ago
suite/browser/tabbrowser.xml also only does an exact match.
(In reply to neil@parkwaycc.co.uk from comment #2)
> suite/browser/tabbrowser.xml also only does an exact match.

http://mxr.mozilla.org/comm-central/ident?i=onLinkAdded
*has "same" regex as SM initFeedTab()...
*FF calls isValidFeed() instead...
 Should SM do that too?

Comment 4

6 years ago
Sorry, but I don't want tabbrowser.xml depending on utilityOverlay.js
Keywords: helpwanted
Not a regression...
tracking-seamonkey2.7: ? → -
status-seamonkey2.7: affected → wontfix
status-seamonkey2.9: --- → affected
(Assignee)

Comment 6

5 years ago
Created attachment 685054 [details] [diff] [review]
Include whitespace in test. (v1)
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #685054 - Flags: review?(neil)

Comment 7

5 years ago
Comment on attachment 685054 [details] [diff] [review]
Include whitespace in test. (v1)

>+                          /^(\s*)application\/(?:atom|rss)\+xml(\s*)$/i.test(type));
You don't need the ()s around \s*. r=me with that fixed.
isValidFeed actually accepts slightly more, it looks like this:
^\s*application\/(?:atom|rss)\+xml\s*(?:;.*)?$
Attachment #685054 - Flags: review?(neil) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 685070 [details] [diff] [review]
Include whitespace in test. (v2)
Attachment #685054 - Attachment is obsolete: true
Attachment #685070 - Flags: review+
(Assignee)

Comment 9

5 years ago
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/a74f2bbce842
Comment on attachment 685070 [details] [diff] [review]
Include whitespace in test. (v2)

[Triage Comment]
Attachment #685070 - Flags: approval-comm-beta+
Attachment #685070 - Flags: approval-comm-aurora+
(Assignee)

Comment 11

5 years ago
Pushed to Comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/11e470198aae

Comment 12

5 years ago
Can we remove the other todo checks in feed_discovery.html and test_feed_discovery.html now that we don't have any more todo tests here?
Flags: needinfo?

Comment 13

5 years ago
(In reply to Edmund Wong (:ewong) from comment #9)
> Pushed to comm-central:
> http://hg.mozilla.org/comm-central/rev/a74f2bbce842

-    <link rel="alternate" type="  application/atom+xml  " title="todo11" href="/11.atom" />
+    <link rel="alternate" type="  application/atom+xml  " title="10" href="/11.atom" />

Should be 'title="11"'?
Flags: needinfo?

Comment 14

5 years ago
> Should be 'title="11"'?
Yes you're right :P
(Assignee)

Comment 15

5 years ago
Created attachment 685127 [details] [diff] [review]
Test fix for patch. (v1)

rs=Ratty over irc.
Attachment #685127 - Flags: review+

Comment 16

5 years ago
Comment on attachment 685127 [details] [diff] [review]
Test fix for patch. (v1)

[Triage Comment]
Attachment #685127 - Flags: approval-comm-beta+
Attachment #685127 - Flags: approval-comm-aurora+
(Assignee)

Comment 17

5 years ago
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/159102806630
(Assignee)

Comment 18

5 years ago
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/eb871ba08465
(Assignee)

Comment 19

5 years ago
Pushed to comm-beta:
http://hg.mozilla.org/releases/comm-beta/rev/5d098b1861bf
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
status-seamonkey2.15: --- → fixed
status-seamonkey2.16: --- → fixed
status-seamonkey2.17: --- → fixed
Target Milestone: --- → seamonkey2.17
(Assignee)

Updated

5 years ago
status-seamonkey2.8: affected → wontfix
status-seamonkey2.9: affected → wontfix
(Assignee)

Updated

5 years ago
Keywords: helpwanted
You need to log in before you can comment on or make changes to this bug.